Crash on receiving geolocation

[mmbd]

How to reproduce

1. I have prepared a minimal working example to reproduce this bug here.
2. Try to publish the user location from XEP-0080 Example 1 (No link since new users can only use 2 links) using a different client. I used the XML console of gajim.

Info

• Smack version 4.4.0 (4.4.0-4.4 2020-12-06)
• Debug log debug.txt (10.4 KB)
• Stack trace:

java.lang.IllegalStateException: Illegal to call getName() when event type is CHARACTERS. Valid states are START_ELEMENT, END_ELEMENT
        at java.xml/com.sun.org.apache.xerces.internal.impl.XMLStreamReaderImpl.getName(XMLStreamReaderImpl.java:998)
        at org.jivesoftware.smack.xml.stax.StaxXmlPullParser.getQName(StaxXmlPullParser.java:124)
        at org.jivesoftware.smack.xml.stax.StaxXmlPullParser.getName(StaxXmlPullParser.java:118)
        at org.jivesoftware.smackx.pubsub.provider.ItemProvider.parse(ItemProvider.java:59)
        at org.jivesoftware.smackx.pubsub.provider.ItemProvider.parse(ItemProvider.java:44)
        at org.jivesoftware.smack.provider.Provider.parse(Provider.java:53)
        at org.jivesoftware.smack.util.PacketParserUtils.parseExtensionElement(PacketParserUtils.java:834)
        at org.jivesoftware.smack.util.PacketParserUtils.addExtensionElement(PacketParserUtils.java:933)
        at org.jivesoftware.smack.util.PacketParserUtils.addExtensionElement(PacketParserUtils.java:928)
        at org.jivesoftware.smack.provider.EmbeddedExtensionProvider.parse(EmbeddedExtensionProvider.java:104)
        at org.jivesoftware.smack.provider.EmbeddedExtensionProvider.parse(EmbeddedExtensionProvider.java:85)
        at org.jivesoftware.smack.provider.Provider.parse(Provider.java:53)
        at org.jivesoftware.smack.util.PacketParserUtils.parseExtensionElement(PacketParserUtils.java:834)
        at org.jivesoftware.smack.util.PacketParserUtils.addExtensionElement(PacketParserUtils.java:933)
        at org.jivesoftware.smack.util.PacketParserUtils.addExtensionElement(PacketParserUtils.java:928)
        at org.jivesoftware.smack.provider.EmbeddedExtensionProvider.parse(EmbeddedExtensionProvider.java:104)
        at org.jivesoftware.smack.provider.EmbeddedExtensionProvider.parse(EmbeddedExtensionProvider.java:85)
        at org.jivesoftware.smack.provider.Provider.parse(Provider.java:53)
        at org.jivesoftware.smack.util.PacketParserUtils.parseExtensionElement(PacketParserUtils.java:834)
        at org.jivesoftware.smack.util.PacketParserUtils.parseMessage(PacketParserUtils.java:186)
        at org.jivesoftware.smack.util.PacketParserUtils.parseStanza(PacketParserUtils.java:111)
        at org.jivesoftware.smack.AbstractXMPPConnection.parseAndProcessStanza(AbstractXMPPConnection.java:1448)
        at org.jivesoftware.smack.tcp.XMPPTCPConnection.access$1000(XMPPTCPConnection.java:130)
        at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.parsePackets(XMPPTCPConnection.java:969)
        at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.access$700(XMPPTCPConnection.java:913)
        at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:936)
        at java.base/java.lang.Thread.run(Thread.java:833)

[Flow]

Does this also happen with Smack 4.4.4?

[mmbd]

Yes, I get the same error with Smack 4.4.4:
debug2.txt (9.6 KB)

[mmbd]

The issue seems to be in the org.jivesoftware.smackx.pubsub.provider.ItemProvider class and is caused by whitespace in the item. For example receiving following item does not result in a crash:

<message type='headline' to='[SCRUBBED]@wiuwiu.de/n14srVRN' from='[SCRUBBED]@wiuwiu.de'>
  <event xmlns='http://jabber.org/protocol/pubsub#event'>
    <items node='http://jabber.org/protocol/geoloc'>
      <item id='6216708a-7c2b-4758-9b9c-415df6c5f0d0'>
        <geoloc xmlns='http://jabber.org/protocol/geoloc' xml:lang='en'/>
      </item>
    </items>
  </event>
</message>

However if whitespace (text inside the item) is present, Smack crashes:

<message type='headline' to='[SCRUBBED]@wiuwiu.de/n14srVRN' from='[SCRUBBED]@wiuwiu.de'>
  <event xmlns='http://jabber.org/protocol/pubsub#event'>
    <items node='http://jabber.org/protocol/geoloc'>
      <item id='13a3710c-68c3-4da2-8484-d8d9c77af91e'>
         
        <geoloc xmlns='http://jabber.org/protocol/geoloc' xml:lang='en'/>
      </item>
    </items>
  </event>
</message>

Note the extra line between item and geoloc. It seems that ItemProvider.parse does not expect to read whitespace (or text) inside the item.

[Flow]

If the child of an XML element consists of text and further elements, then this is called “mixed content”. XMPP explicitly forbids mixed content in a few places (cf. RFC 6121 § 5.2.3) and I believe that your with the mixed content of a whitespace-only text and an <geoloc/> element would not pass XEP-0060’s schema definition. Furthermore, it is unclear how the textual part of the mixed content can be used, or even, if it should be used.

On the other hand, the schema of a XEP is largely seen as informal and I can’t find any formal explicit statement that forbids mixed content here.

I am currently torn on this. My policy is to not add changes to Smack that support non-standard behavior. At least not as part of my spare time work on Smack, and not without those being configurable and default to off. I may have to think about this a little longer, but right now, I lean towards not changing Smack into accepting this.

Do you have any information about the implementation on the orginating entity that procudes this <item/>? Irregardless of Smack accepting or not accepting this, you may want to inform the people behind the implementation to change this behavior.

[guus]

Is the order/frequency in which white space characters (newlines, spaces, tabs) appear in between elements relevant, when there are no other characters than white space characters? In other words: is newline space space significantly different from newline space space newline space space in that regard?

As an aside, we identified a very similar issue in Openfire not to long ago: Openfire - Issues - Ignite Realtime Jira

[Flow]

@guus

Is the order/frequency in which white space characters (newlines, spaces, tabs) appear in between elements relevant, when there are no other characters than white space characters?

I don’t think so, the order is not relevant. But from XML’s perspective, they all, newlines, spaces, tabs, all other whitespace and non-whitespace characters, add character data to the element’s content.

[guus]

My question only makes much sense if the original stanza, that is not causing the issue, is being exchanged with Smack exactly as shown here. If what is shown here on Discourse is a pretty-printed representation of data that was transmitted without any mixed content, then my question is primarily adding confusion.

[mmbd]

@Flow

Do you have any information about the implementation on the orginating entity that procudes this ? Irregardless of Smack accepting or not accepting this, you may want to inform the people behind the implementation to change this behavior.

The orginating entity in this case would be the Prosody PEP service. I feel like whitespace should just be ignored (i.e. only throw an exception when non-whitespace is encountered).

@guus

If what is shown here on Discourse is a pretty-printed representation of data that was transmitted without any mixed content, then my question is primarily adding confusion.

Both stanzas are copy-pasted from the output I get when I set SmackConfiguration.DEBUG = true.

[Flow]

@mmbd

The orginating entity in this case would be the Prosody PEP service.

I could also imagine that Prosody just shoves “items” that are published into the node verbatim. Hence the causing entity could be the item publishing one.

@mmbd

I feel like whitespace should just be ignored (i.e. only throw an exception when non-whitespace is encountered).

The XML level makes no distinction if it’s whitespace or not. It’s just character data, together with an element, and hence mixed content.

That said, I’ve just created SMACK-919 for this.

[Flow]

[pubsub] Allow for character data before <item/>'s payload

[guus]

I’m noticing this exception in my logs after updating Spark to Smack 4.4.4. I’ve redacted the JIDs to protect the innocent. Is this related?

java.lang.IllegalStateException: Illegal to call getName() when event type is CHARACTERS. Valid states are START_ELEMENT, END_ELEMENT
at com.sun.org.apache.xerces.internal.impl.XMLStreamReaderImpl.getName(XMLStreamReaderImpl.java:938)
	at org.jivesoftware.smack.xml.stax.StaxXmlPullParser.getQName(StaxXmlPullParser.java:123)
	at org.jivesoftware.smack.xml.stax.StaxXmlPullParser.getName(StaxXmlPullParser.java:117)
	at org.jivesoftware.sparkimpl.profile.ext.VCardUpdateExtension$Provider.parse(VCardUpdateExtension.java:75)
	at org.jivesoftware.sparkimpl.profile.ext.VCardUpdateExtension$Provider.parse(VCardUpdateExtension.java:62)
	at org.jivesoftware.smack.provider.Provider.parse(Provider.java:53)
	at org.jivesoftware.smack.util.PacketParserUtils.parseExtensionElement(PacketParserUtils.java:834)
	at org.jivesoftware.smack.util.PacketParserUtils.parsePresence(PacketParserUtils.java:477)
	at org.jivesoftware.smack.util.PacketParserUtils.parseStanza(PacketParserUtils.java:115)
	at org.jivesoftware.smack.AbstractXMPPConnection.parseAndProcessStanza(AbstractXMPPConnection.java:1457)
	at org.jivesoftware.smack.tcp.XMPPTCPConnection.access$1000(XMPPTCPConnection.java:130)
	at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.parsePackets(XMPPTCPConnection.java:969)
	at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.access$700(XMPPTCPConnection.java:913)
	at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:936)
	at java.lang.Thread.run(Thread.java:748)

Sent by the Psi client, apparently.

[Flow]

It’s hard to tell if it’s fixed by SMACK-919 without a stacktrace that provides some context where this is happening. It is likely related in the sense that this is the same class of coding error (which should be trivial to fix).

[guus]

Sorry, I’m running in and out of meetings without properly giving this the details that it needs. I’ve edited my previous message to include the stacktrace.

[Flow]

[core] Fix VCardUpdateExtension.Provider