[muc] Bookmarks.getConferenceStorage() don't parse empty <nick/>

[stokito]

For a conference a server may return empty element.
This will cause an exception on parsing and the whole conference won't appear in a client. A similar problem will occur if something wrong with the nick format. The nick in a conference is optional thing, and we are safe to ignore any problems with it. In this case a default account JID must be used.

Strictly speaking this change may potentially cause NPE in other places. I found that there are some checks for the Nickname to be empty in the
org/jivesoftware/smackx/muc/bookmarkautojoin/MucBookmarkAutojoinManager.java:125:

            Resourcepart nick = bookmarkedConference.getNickname();
            if (nick == null) {
                nick = defaultNick;
            }

The second place where the getNickname() is called is a serialization method toXML and I made it not to print the <nick>null</nick> element.
So I hope no one will be affected by the change or at least the problem will be now better localized.

I do have the problem IRL:

<iq xmlns="jabber:client" xml:lang="en" to="stokito@conversations.im/gajim.K4OZSDIV" from="stokito@conversations.im" type="result" id="adc7083e-b00a-45b8-98cd-bf31a8bfb2c3">
  <pubsub xmlns="http://jabber.org/protocol/pubsub">
    <items node="urn:xmpp:bookmarks:1">
      <item id="ru@chat.404.city">
        <conference xmlns="urn:xmpp:bookmarks:1" name="RuChat:  Чат русско-язычного XMPP" autojoin="false">
          <nick />
          <extensions />
</conference>

So here I joined a conference ru@chat.404.city from my account stokito@conversations.im from Gajim. It looks like I didn't specified my nickname and on the server it was set to empty.

The XEP-0402 says that the whole nick element is optional. This is an issue of the XEP that didn't clearly specified this and an XMPP server that it mustn't return the empty nick element. Both 404.city and conversations.im use latest ejaberd and I'll report a corresponding bug there. UPD processone/ejabberd#4272

Still we need this check in the Smack to avoid problems with old versions and other badly implemented servers.

[Flowdalic]

Thanks for your contribution to Smack.

This is a little bit convoluted, but basically, the nick in a MUC must be able to be mapped to a Resourcepart and Resourceparts must not be the empty string. Therefore, Smack rejects <nick/> with the empty string as invalid.

Event though not explicitly spelled out by XEP-402 (and XEP-172), which I consider something the XEPs should do, an <nick/> containing an empty string is non-compliant and Smack will reject the whole stanza.

If it wouldn't reject the whole Stanza in such cases, then, even considering that the faulty part is optional, this would cause inconsistent assumptions between the sender and the recipient(s). And, due the principle of explosion, all kinds of errors could occur due to that.

I suggest to contact the authors of implementations that emit an <nick/> with an emtpy String and ask them to fix this.

[stokito]

UPD Reported to Gajim https://dev.gajim.org/gajim/gajim/-/issues/11980

For now the empty nick causes the Spark to not show the conference at all. So I can't even leave the conference.
Maybe the check for a Nick empty can be done on a higher level of the app.
I don't know how critical it can be. From my understanding if we ignore the empty nick then user will be able to do at least something which is better than nothing.
Feel free to close the pr if you think otherwise.

[Flowdalic]

From my understanding if we ignore the empty nick then user will be able to do at least something which is better than nothing.

Which falls into the robustness principle, which is believed by many, including me, to be a mistake these days. See https://datatracker.ietf.org/doc/html/rfc9413#name-harmful-consequences-of-tol