Skip navigation
1242 Views 8 Replies Latest reply: Mar 31, 2012 6:14 AM by Flow RSS
colby.white Bronze 3 posts since
Mar 26, 2012
Currently Being Moderated

Mar 26, 2012 8:56 AM

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.

Attachments:
  • Daryl Herzmann KeyContributor 1,534 posts since
    Mar 12, 2005
    Currently Being Moderated
    Mar 26, 2012 10:17 AM (in response to colby.white)
    MUC ownership doesn't follow current xep spec

    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.

  • rcollier KeyContributor 982 posts since
    Mar 4, 2009
    Currently Being Moderated
    Mar 26, 2012 12:57 PM (in response to colby.white)
    MUC ownership doesn't follow current xep spec

    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.

      • Daryl Herzmann KeyContributor 1,534 posts since
        Mar 12, 2005
        Currently Being Moderated
        Mar 27, 2012 4:59 AM (in response to colby.white)
        MUC ownership doesn't follow current xep spec

        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.

      • rcollier KeyContributor 982 posts since
        Mar 4, 2009
        Currently Being Moderated
        Mar 27, 2012 11:14 AM (in response to colby.white)
        MUC ownership doesn't follow current xep spec

        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

          • rcollier KeyContributor 982 posts since
            Mar 4, 2009
            Currently Being Moderated
            Mar 28, 2012 1:39 PM (in response to colby.white)
            MUC ownership doesn't follow current xep spec

            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).

  • Flow KeyContributor 605 posts since
    Jan 11, 2007
    Currently Being Moderated
    Mar 31, 2012 6:14 AM (in response to colby.white)
    Re: MUC ownership doesn't follow current xep spec

    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

More Like This

  • Retrieving data ...

Bookmarked By (0)