Merge branch '4.4'

smack-extensions/src/main/java/org/jivesoftware/smackx/caps/EntityCapsManager.java

@@ -23,6 +23,7 @@ import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
@@ -701,16 +702,30 @@ public final class EntityCapsManager extends Manager {
         }
 
         List<DataForm> extendedInfos = discoverInfo.getExtensions(DataForm.class);
-        for (DataForm extendedInfo : extendedInfos) {
+        final Iterator<DataForm> iter = extendedInfos.iterator();
-            if (!extendedInfo.hasHiddenFormTypeField()) {
+        while (iter.hasNext()) {
+            if (!iter.next().hasHiddenFormTypeField()) {
                 // Only use the data form for calculation is it has a hidden FORM_TYPE field.
                 // See XEP-0115 5.4 step 3.f
-                continue;
+                iter.remove();
             }
+        }
 
-            // 6. If the service discovery information response includes
+        // 6. If the service discovery information response includes
-            // XEP-0128 data forms, sort the forms by the FORM_TYPE (i.e.,
+        // XEP-0128 data forms, sort the forms by the FORM_TYPE (i.e.,
-            // by the XML character data of the <value/> element).
+        // by the XML character data of the <value/> element).
+        Collections.sort(extendedInfos, new Comparator<DataForm>() {
+            @Override
+            public int compare(DataForm dataFormLeft, DataForm dataFormRight) {
+                final String formTypeLeft = dataFormLeft.getFormType();
+                assert formTypeLeft != null; // ensured by the previous step.
+                final String formTypeRight = dataFormRight.getFormType();
+                assert formTypeRight != null; // ensured by the previous step.
+                return formTypeLeft.compareTo(formTypeRight);
+            }
+        });
+
+        for (DataForm extendedInfo : extendedInfos) {
             SortedSet<FormField> fs = new TreeSet<>(new Comparator<FormField>() {
                 @Override
                 public int compare(FormField f1, FormField f2) {

smack-extensions/src/test/java/org/jivesoftware/smackx/caps/EntityCapsManagerTest.java

@@ -47,6 +47,41 @@ import org.jxmpp.stringprep.XmppStringprepException;
 
 public class EntityCapsManagerTest extends SmackTestSuite {
 
+    /**
+     * <a href="https://xmpp.org/extensions/xep-0115.html#ver-gen-simple">XEP-
+     * 0115 Simple Generation Example</a>.
+     * @throws XmppStringprepException if the provided string is invalid.
+     */
+    @Test
+    public void testSimpleGenerationExample() throws XmppStringprepException {
+        DiscoverInfo di = createSimpleSamplePacket();
+
+        CapsVersionAndHash versionAndHash = EntityCapsManager.generateVerificationString(di, StringUtils.SHA1);
+        assertEquals("QgayPKawpkPSDYmwT/WM94uAlu0=", versionAndHash.version);
+    }
+
+    /**
+     * Asserts that the order in which data forms are present in the disco/info does not affect the calculated
+     * verification string, as the XEP mandates that these are ordered by FORM_TYPE (i.e., by the XML character data of
+     * the <value/> element).
+     * @throws XmppStringprepException if the provided string is invalid.
+     */
+    @Test
+    public void testReversedDataFormOrder() throws XmppStringprepException {
+        final DiscoverInfoBuilder builderA = createSimpleSampleBuilder();
+        builderA.addExtension(createSampleServerInfoDataForm()); // This works, as the underlying MultiMap maintains insertion-order.
+        builderA.addExtension(createSampleSoftwareInfoDataForm());
+
+        final DiscoverInfoBuilder builderB = createSimpleSampleBuilder();
+        builderB.addExtension(createSampleSoftwareInfoDataForm());
+        builderB.addExtension(createSampleServerInfoDataForm());
+
+        CapsVersionAndHash versionAndHashA = EntityCapsManager.generateVerificationString(builderA.build(), StringUtils.SHA1);
+        CapsVersionAndHash versionAndHashB = EntityCapsManager.generateVerificationString(builderB.build(), StringUtils.SHA1);
+
+        assertEquals(versionAndHashA.version, versionAndHashB.version);
+    }
+
     /**
      * <a href="http://xmpp.org/extensions/xep-0115.html#ver-gen-complex">XEP-
      * 0115 Complex Generation Example</a>.
@@ -142,6 +177,41 @@ public class EntityCapsManagerTest extends SmackTestSuite {
         return df.build();
     }
 
+    private static DataForm createSampleServerInfoDataForm() {
+        DataForm.Builder df = DataForm.builder(DataForm.Type.result);
+
+        {
+            TextMultiFormField.Builder ff = FormField.textMultiBuilder("admin-addresses");
+            ff.addValue("xmpp:admin@example.org");
+            ff.addValue("mailto:admin@example.com");
+            df.addField(ff.build());
+        }
+
+        {
+            TextSingleFormField.Builder ff = FormField.hiddenBuilder("FORM_TYPE");
+            ff.setValue("http://jabber.org/network/serverinfo");
+            df.addField(ff.build());
+        }
+
+        return df.build();
+    }
+
+    private static DiscoverInfoBuilder createSimpleSampleBuilder() throws XmppStringprepException {
+        DiscoverInfoBuilder di = DiscoverInfo.builder("disco1");
+        di.ofType(IQ.Type.result);
+
+        di.addIdentity(new DiscoverInfo.Identity("client", "Exodus 0.9.1", "pc"));
+        di.addFeature("http://jabber.org/protocol/disco#info");
+        di.addFeature("http://jabber.org/protocol/disco#items");
+        di.addFeature("http://jabber.org/protocol/muc");
+        di.addFeature("http://jabber.org/protocol/caps");
+
+        return di;
+    }
+    private static DiscoverInfo createSimpleSamplePacket() throws XmppStringprepException {
+        return createSimpleSampleBuilder().build();
+    }
+
     private static DiscoverInfo createComplexSamplePacket() throws XmppStringprepException {
         DiscoverInfoBuilder di = DiscoverInfo.builder("disco1");
         di.from(JidCreate.from("benvolio@capulet.lit/230193"));