MUC ownership doesn't follow current xep spec

According to XEP-0045, getting a list of owners for a MUC and granting ownership for a MUC are done with an iq stanza that contains a query object in the muc#admin namespace. Please see examples 181 and 183 in the section of XEP-0045 regarding modifying the owner list.

Currently, Smack implements those two actions with a muc#owner namespace. Smack uses private methods MultiUserChat.changeAffiliationByOwner and MultiUserChat.getAffiliatesByOwner for the two actions. Both methods create iqs of the type MUCOwner (which prints out a muc#owner namespace). These methods need to be changed to iqs of type MUCAdmin to properly follow the spec. That appears to be an easy fix since other actions that these methods are used for (granting admin rights, get a list of admins) also need the muc#admin namespace. I haven’t done thorough analysis of the MultiUserChat to see all that those methods do; just enough to debug my ownership related problem. So that statement of it being an easy fix could be wrong.

After doing some digging, it seems the root of the problem is the fact that these actions were actually in the muc#owner namespace back in an older version of the XEP-0045 spec. To be specific, these actions changed to the muc#admin namespace on version 1.19 of the xep-0045 spec, which was apparently published in April 2005 (and refered to as jep-0045). Afterward, it appears most server code still allowed the muc#owner namespace for legacy reasons, which is why, for most servers, Smack still can perform the actions with the muc#owner namespace without any problems. But for newer server code - namely Prosody, which is what I was using when this bug presented and was created in Aug. 2008 - Smack doesn’t work properly.

You can test this by creating a MUC on a prosody server (i.e. neko.im, thiessen.im, or your own local server), then trying to view the owner list. It shouldn’t work and didn’t for me. This is because prosody doesn’t recognize the muc#owner. And, according to the spec, it shouldn’t.

Again, the fix is to use the MUCAdmin type to get the muc#admin namespace. I’m currently doing this with my own MUCHack class, which is inculded below. For the most part, I simply copy-pasted the two methods I mentioned earlier and changed the MUCOwner to MUCAdmin. It works fine on both prosody and non-prosody servers. (Again, I haven’t done thorough testing.)

A couple notes on MUCHack:

  • I implemented it via static methods for my own purposes.

  • the constructor for the Affiliate class only has package visibility; so MUCHack has to be in the org.jivesoftware.smackx.muc package.

public class MUCHack {
    @SuppressWarnings("rawtypes")
    public static Collection<Affiliate> getAffiliates(MultiUserChat muc,XMPPConnection con,String affiliation) throws XMPPException{
        MUCAdmin iq = new MUCAdmin();
        iq.setTo(muc.getRoom());
        iq.setType(IQ.Type.GET);
        // Set the specified affiliation. This may request the list of owners/admins/members/outcasts.
        MUCAdmin.Item item = new MUCAdmin.Item(affiliation,null);
        iq.addItem(item);         // Wait for a response packet back from the server.
        PacketFilter responseFilter = new PacketIDFilter(iq.getPacketID());
        PacketCollector response = con.createPacketCollector(responseFilter);
        // Send the request to the server.
        con.sendPacket(iq);
        // Wait up to a certain number of seconds for a reply.
        MUCAdmin answer = (MUCAdmin) response.nextResult(SmackConfiguration.getPacketReplyTimeout());
        // Stop queuing results
        response.cancel();         if (answer == null) {
            throw new XMPPException("No response from server.");
        }
        else if (answer.getError() != null) {
            throw new XMPPException(answer.getError());
        }
        // Get the list of affiliates from the server's answer
        List<Affiliate> affiliates = new ArrayList<Affiliate>();
        for (Iterator it = answer.getItems(); it.hasNext();) {
            affiliates.add(new Affiliate((MUCAdmin.Item) it.next()));
        }
        return affiliates;
    }     public static void grantAffiliation(MultiUserChat muc,XMPPConnection con,String affiliation,String jid) throws XMPPException{
        MUCAdmin iq = new MUCAdmin();
        iq.setTo(muc.getRoom());
        iq.setType(IQ.Type.SET);
        // Set the new affiliation.
        MUCAdmin.Item item = new MUCAdmin.Item(affiliation,null);
        item.setJid(jid);
        iq.addItem(item);         // Wait for a response packet back from the server.
        PacketFilter responseFilter = new PacketIDFilter(iq.getPacketID());
        PacketCollector response = con.createPacketCollector(responseFilter);
        // Send the change request to the server.
        con.sendPacket(iq);
        // Wait up to a certain number of seconds for a reply.
        IQ answer = (IQ) response.nextResult(SmackConfiguration.getPacketReplyTimeout());
        // Stop queuing results
        response.cancel();         if (answer == null) {
            throw new XMPPException("No response from server.");
        }
        else if (answer.getError() != null) {
            throw new XMPPException(answer.getError());
        }
    }
}

On a sidenote, this part of the spec changed 7 years ago. Is there not a whole lot of work being done on the Smack side to stay with updates in the spec? Is this common with all libraries? common with XMPP as a whole? I’m not sure how often the spec updates or how big the changes are when there are updates. (If there aren’t many changes, code being on an older spec doesn’t really pose an issue I guess.) Just trying to figure out how big of a deal this is. 7 years makes me nervous.
MUCHack.java.zip (948 Bytes)

1 Like

Indeed, the openfire MUC implemention is that old and has yet to be updated: OF-178 . Smack is probably coded to support openfire’s implementation.

I share your nervousness, although in my case it is especially about changing the code and potentially breaking a lot of existing code if the servers are not updated to (which they are apparently not). I think I am going to have to make sure that a solution remains backward compatible as well, unless most servers themselves have updated but remain backward compatible, which may be hard to confirm.

I find this to be a bit of an issue with XMPP where specs can be in draft state for years and slowly change. It’s hard to tell what the state of affairs is for the various servers that exist and how much code could be broken if things aren’t kept backword compatible.

It would be nice to be able to have some handshaking for version information to allow code to adapt to differing versions. I thought it was peculiar when I first starting looking at XMPP servers that they advertise what extensions they support, but not which versions. Without the version information, you can’t tell what features may be missing from the implementation.

What’s the point of the spec if no one’s going to follow them? Understand what I’ve got right now: a handful of spec-abiding servers that I can’t use because smack doesn’t follow the spec. Now you’re saying/implying Smack won’t follow the spec because it’ll break servers that don’t follow the spec.

so … what’s the point of the spec? Contrary to popular belief, rules are not meant to be broken. Cuz this is what happens.

I realize I’m exaggerating the severity of this particular bug, but I’m sure you see the frustration and the possibilities of problems not following the spec could If Smack isn’t following the spec because of OpenFire, then shouldn’t OpenFire be updated support the spec?

On another note, there appears to be an implementation of your version idea rcollier. http://bit.ly/H76CEY . I’m not sure excatly how that’s implemented in practice.

Colby, The problem is that we have no active openfire developers, just a few of us attempting to squash small bugs. Major efforts like bringing openfire up to date with the most recent MUC Spec is something we’d need some community member to take charge of and generate a patch.

I am not implying that Smack won’t follow the spec (I am very much a proponent of sticking to the spec), I am saying it has to be backward compatible so existing code will still work. It would be great to simply say that it will simply follow the current version of the spec, but breaking everyones code if they update is not a viable option either. It would be simpler, but we would probably lose a lot of users (or at least piss them off ).

I do thank you for bringing this up though. I think it is a high priority item since it fails to actually work with some servers.

Logged as SMACK-371

I understand the ‘backwards-compatibility’ philosophy, but I’ve always been of the mindset if a spec changes, everyone needs to change. Trying to support b-c creates more problems than it solves sometimes in my opinion. I argue (in this community and others) that this is a case of that since it endorses ignoring the spec, which then defeats the purpose of the spec.

Cleary I’m the minority opinion here (in other spaces, I may not be the minority, but no ones going to do anything because they’re scared of b-c endorsers. haha) This backwards-compatibility debate is as old as the Z1. If no one’s won that debate yet, me and you aren’t going to fare any better .

But, my main point (which I didn’t clearly articulate in my previous post) is 7 years is a whole lot of backwards-compatibility. Especially in the software industry.

1 Like

I can’t argue with that last statement. Of course the fact that the spec is still in Draft doesn’t help much either .

Usually the specs don’t break b-c, but in some cases they do due to implementations showing problems or shortcomings with the previous approach.

I would have no problem making the change if I knew that most servers are updated to the newer (and by newer I mean not ancient) version of the spec. I don’t mind pissing off a few people, just not everyone. I would love to say that I would update Openfire as well, but I only have so much free time on my hands (lately, that has amounted to 0).

Good finding!

But smack already provides getAffilatesByAdmin(). So wouldn’t it be enought to simply change MulitUserChat.java:1635 to use this function instead of getAffilatesByOwner()? And do the same for all other Onwer related methods, since changeAffiliationByAdmin() also exists.

I have done this in my fork’s branch smack_extended used by asmack, the change can be viewed here: https://github.com/Flowdalic/smack/commit/147a4b03bcde6520949308a21edee5520bc202 83

It needs testing. I am thankfull for feedback about the compatibility with different XMPP servers.

Flow