[xdata] Do not lookup fields of forms without FORM_TYPE in registry

igniterealtime · Feb 21, 2021 · d7f47c8 · d7f47c8
1 parent ce4f85a
commit d7f47c8
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 65 deletions.
diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/formtypes/FormFieldRegistry.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/formtypes/FormFieldRegistry.java
@@ -1,6 +1,6 @@
 /**
  *
- * Copyright 2020 Florian Schmaus
+ * Copyright 2020-2021 Florian Schmaus
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,8 +19,6 @@
 import java.util.HashMap;
 import java.util.Map;
 
-import org.jivesoftware.smack.util.Objects;
-
 import org.jivesoftware.smackx.xdata.FormField;
 import org.jivesoftware.smackx.xdata.TextSingleFormField;
 import org.jivesoftware.smackx.xdata.packet.DataForm;
@@ -29,14 +27,8 @@ public class FormFieldRegistry {
 
     private static final Map<String, Map<String, FormField.Type>> REGISTRY = new HashMap<>();
 
-    private static final Map<String, FormField.Type> LOOKASIDE_REGISTRY = new HashMap<>();
-
     private static final Map<String, String> FIELD_NAME_TO_FORM_TYPE = new HashMap<>();
 
-    static {
-        register(FormField.FORM_TYPE, FormField.Type.hidden);
-    }
-
     @SuppressWarnings("ReferenceEquality")
     public static synchronized void register(DataForm dataForm) {
         // TODO: Also allow forms of type 'result'?
@@ -46,8 +38,8 @@ public static synchronized void register(DataForm dataForm) {
 
         String formType = null;
         TextSingleFormField hiddenFormTypeField = dataForm.getHiddenFormTypeField();
-        if (hiddenFormTypeField != null) {
-            formType = hiddenFormTypeField.getValue();
+        if (hiddenFormTypeField == null) {
+            throw new IllegalArgumentException("Can only register forms with (hidden) FROM_TYPE field");
         }
 
         for (FormField formField : dataForm.getFields()) {
@@ -63,25 +55,6 @@ public static synchronized void register(DataForm dataForm) {
     }
 
     public static synchronized void register(String formType, String fieldName, FormField.Type type) {
-        if (formType == null) {
-            FormFieldInformation formFieldInformation = lookup(fieldName);
-            if (formFieldInformation != null) {
-                if (Objects.equals(formType, formFieldInformation.formType)
-                                && type.equals(formFieldInformation.formFieldType)) {
-                    // The field is already registered, nothing to do here.
-                    return;
-                }
-
-                String message = "There is already a field with the name'" + fieldName
-                                + "' registered with the field type '" + formFieldInformation.formFieldType
-                                + "', while this tries to register the field with the type '" + type + '\'';
-                throw new IllegalArgumentException(message);
-            }
-
-            LOOKASIDE_REGISTRY.put(fieldName, type);
-            return;
-        }
-
         Map<String, FormField.Type> fieldNameToType = REGISTRY.get(formType);
         if (fieldNameToType == null) {
             fieldNameToType = new HashMap<>();
@@ -97,31 +70,8 @@ public static synchronized void register(String formType, String fieldName, Form
         FIELD_NAME_TO_FORM_TYPE.put(fieldName, formType);
     }
 
-    public static synchronized void register(String fieldName, FormField.Type type) {
-        FormField.Type previousType = LOOKASIDE_REGISTRY.get(fieldName);
-        if (previousType != null) {
-            if (previousType == type) {
-                // Nothing to do here.
-                return;
-            }
-            throw new IllegalArgumentException("There is already a field with the name '" + fieldName
-                            + "' registered with type " + previousType
-                            + ", while trying to register this field with type '" + type + "'");
-        }
-
-        LOOKASIDE_REGISTRY.put(fieldName, type);
-    }
-
     public static synchronized FormField.Type lookup(String formType, String fieldName) {
-        if (formType != null) {
-            Map<String, FormField.Type> fieldNameToTypeMap = REGISTRY.get(formType);
-            if (fieldNameToTypeMap != null) {
-                FormField.Type type = fieldNameToTypeMap.get(fieldName);
-                if (type != null) {
-                    return type;
-                }
-            }
-        } else {
+        if (formType == null) {
             formType = FIELD_NAME_TO_FORM_TYPE.get(fieldName);
             if (formType != null) {
                 FormField.Type type = lookup(formType, fieldName);
@@ -131,8 +81,15 @@ public static synchronized FormField.Type lookup(String formType, String fieldNa
             }
         }
 
-        // Fallback to lookaside registry.
-        return LOOKASIDE_REGISTRY.get(fieldName);
+        Map<String, FormField.Type> fieldNameToTypeMap = REGISTRY.get(formType);
+        if (fieldNameToTypeMap != null) {
+            FormField.Type type = fieldNameToTypeMap.get(fieldName);
+            if (type != null) {
+                return type;
+            }
+        }
+
+        return null;
     }
 
     public static synchronized FormFieldInformation lookup(String fieldName) {

diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/packet/DataForm.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/packet/DataForm.java
@@ -105,7 +105,7 @@ private DataForm(Builder builder) {
         extensionElements = CollectionUtil.cloneAndSeal(builder.extensionElements);
 
         // Ensure that the types of the form fields of every data form is known by registering such fields.
-        if (type == Type.form) {
+        if (type == Type.form && hasHiddenFormTypeField()) {
             FormFieldRegistry.register(this);
         }
     }

diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/provider/DataFormProvider.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/provider/DataFormProvider.java
@@ -207,14 +207,19 @@ private static FormField parseField(XmlPullParser parser, XmlEnvironment xmlEnvi
         }
 
         if (type == null) {
-            // If no field type was explicitly provided, then we need to lookup the
-            // field's type in the registry.
-            type = FormFieldRegistry.lookup(formType, fieldName);
-            if (type == null) {
-                LOGGER.warning("The Field '" + fieldName + "' from FORM_TYPE '" + formType
-                                + "' is not registered. Field type is unknown, assuming text-single.");
-                // As per XEP-0004, text-single is the default form field type, which we use as emergency fallback here.
-                type = FormField.Type.text_single;
+            // The field name 'FORM_TYPE' is magic.
+            if (fieldName.equals(FormField.FORM_TYPE)) {
+                type = FormField.Type.hidden;
+            } else {
+                // If no field type was explicitly provided, then we need to lookup the
+                // field's type in the registry.
+                type = FormFieldRegistry.lookup(formType, fieldName);
+                if (type == null) {
+                    LOGGER.warning("The Field '" + fieldName + "' from FORM_TYPE '" + formType
+                                    + "' is not registered. Field type is unknown, assuming text-single.");
+                    // As per XEP-0004, text-single is the default form field type, which we use as emergency fallback here.
+                    type = FormField.Type.text_single;
+                }
             }
         }