Bug in smack? While interacting with IQProvider. It breaks gtalk gateway plugin

Hi there!

(I have posted a properly formatted post at: http://imaginateaqui.net/tmp/email-openfire-bug-smack.html )

I have found which appears to be a bug in the Smack library.

I am not sure why hasn’t anybody noticed before.

You can find it when using the Gtalk gateway plugin for openfire, and

activating the mailnotifications.

When using the plugin, and connecting to gtalk, the connection

register itself for listening for such packets.

ProviderManager.getInstance().addIQProvider(GoogleMailBoxPacket.MAILBOX_ELEMENT, GoogleMailBoxPacket.MAILBOX_NAMESPACE, new GoogleMailBoxPacket.Provider()); ....     conn.addPacketListener(listener, new OrFilter(
                       new PacketTypeFilter(GoogleMailBoxPacket.class),
....

Well, in the file

smack/source/org/jivesoftware/smack/PacketReader.java
there is this

code that deals with external providers:

private IQ parseIQ(XmlPullParser parser) throws Exception {
    ...
    boolean done = false;
    while (!done) {
        int eventType = parser.next();             if (eventType == XmlPullParser.START_TAG) {
                String elementName = parser.getName();
                String namespace = parser.getNamespace();
                if (elementName.equals("error")) {
                ...
                ...
                ...
                else {
                    Object provider = ProviderManager.getInstance().getIQProvider(elementName, namespace);
                    if (provider != null) {
                        if (provider instanceof IQProvider) {
                            iqPacket = ((IQProvider)provider).parseIQ(parser);
                        }
                        else if (provider instanceof Class) {
                            iqPacket = (IQ)PacketParserUtils.parseWithIntrospection(elementName,
                                    (Class)provider, parser);
                        }
                    }
                }
            }
            else if (eventType == XmlPullParser.END_TAG) {
                if (parser.getName().equals("iq")) {
                    done = true;
                }
            }
        }

So, if there is a provider for the current elementName and namespace,

it handles the parser to it.

But, after that it doesn’t set the ‘done’ value to true.

So, when a packet

<iq ... type="result"> <mailbox url="http://mail.google.com/mail" xmlns="google:mail:notify">
  <mail-thread-info ...></mail-thread-info>
  <mail-thread-info ...></mail-thread-info> </mailbox>
</iq>

arrives, it handles the parser to GoogleMailBoxPacket.java.

This file leaves the parser in the next tag after </iq>, whatever it

is.

Now, when a new packet arrives (e.g., a <presence> from one of your

friends), the function still thinks is parsing a <iq> packet, so it

never ends up doing so.

The result: the gateway plugin stops delivering packets to the

client. The packets arrive to the XMPPConnection, but they don’t get

delivered to the client.

It has taken me a while to reproduce it, but after finding it, i think

it’s a valid bug report.

Just applying this diff, things start to work flawlessly again:

diff --git a/source/org/jivesoftware/smack/PacketReader.java b/source/org/jivesoftware/smack/PacketReader.java
index 09009d9..87adb93 100644
--- a/source/org/jivesoftware/smack/PacketReader.java
+++ b/source/org/jivesoftware/smack/PacketReader.java
@@ -595,10 +595,14 @@ class PacketReader {
                     if (provider != null) {
                         if (provider instanceof IQProvider) {
                             iqPacket = ((IQProvider)provider).parseIQ(parser);
+                            // Assume provider has finished parsing the IQ packet
+                            done = true;
                         }
                         else if (provider instanceof Class) {
                             iqPacket = (IQ)PacketParserUtils.parseWithIntrospection(elementName,
                                     (Class)provider, parser);
+                            // Assume provider has finished parsing the IQ packet
+                            done = true;
                         }
                     }
                 }

What does people think? Do you think it’s a valid bug?

Thanks a lot for your attention, and thanks for smack, openfire and

all the software you’ve created!!

  • Gaizka

Howdy! First of all, great research! I did a little more digging and instead of changing Smack (which I think is more or less doing it right), I fixed the GoogleMailBoxPacket, which was not supposed to be looking for iq to stop, it was supposed to be looking for mailbox. =D Fix is in trunk right now and will be in next release!