[xdata] Avoid NPE if form field's name is not set

Do not throw an NPE if an form field without a name, i.e., the 'var' attribute, is received. Thanks to Peter Kaul for reporting this.
Flowdalic · Oct 15, 2024 · d3de2d6 · d3de2d6
1 parent 02d8f53
commit d3de2d6
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 3 deletions.
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
@@ -29,6 +29,7 @@
 
 import org.jivesoftware.smack.packet.XmlEnvironment;
 import org.jivesoftware.smack.parsing.SmackParsingException;
+import org.jivesoftware.smack.parsing.SmackParsingException.RequiredValueMissingException;
 import org.jivesoftware.smack.provider.ExtensionElementProvider;
 import org.jivesoftware.smack.roster.packet.RosterPacket;
 import org.jivesoftware.smack.roster.provider.RosterPacketProvider;
@@ -185,9 +186,8 @@ private static FormField parseField(XmlPullParser parser, XmlEnvironment xmlEnvi
         FormField.Type type = null;
         {
             String fieldTypeString = parser.getAttributeValue("type");
-            if (fieldTypeString != null) {
-                type = FormField.Type.fromString(fieldTypeString);
-            }
+            // FormField.Type.fromString() will return null if its input is null.
+            type = FormField.Type.fromString(fieldTypeString);
         }
 
         List<FormField.Value> values = new ArrayList<>();
@@ -238,6 +238,14 @@ private static FormField parseField(XmlPullParser parser, XmlEnvironment xmlEnvi
             }
         }
 
+        if (type != FormField.Type.fixed && fieldName == null) {
+            String typeString = "unspecified";
+            if (type != null) {
+                typeString = type.toString();
+            }
+            throw new RequiredValueMissingException("The data form field of " + typeString + " type has no 'var' attribute, even though one is required as per XEP-0004 § 3.2");
+        }
+
         if (type == null) {
             // The field name 'FORM_TYPE' is magic.
             if (fieldName.equals(FormField.FORM_TYPE)) {

diff --git a/...extensions/src/test/java/org/jivesoftware/smackx/xdata/provider/DataFormProviderTest.java b/...extensions/src/test/java/org/jivesoftware/smackx/xdata/provider/DataFormProviderTest.java
@@ -17,11 +17,13 @@
 package org.jivesoftware.smackx.xdata.provider;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.io.IOException;
 import java.util.List;
 
 import org.jivesoftware.smack.parsing.SmackParsingException;
+import org.jivesoftware.smack.test.util.SmackTestUtil;
 import org.jivesoftware.smack.util.PacketParserUtils;
 import org.jivesoftware.smack.xml.XmlPullParser;
 import org.jivesoftware.smack.xml.XmlPullParserException;
@@ -30,6 +32,8 @@
 import org.jivesoftware.smackx.xdata.packet.DataForm;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
 
 public class DataFormProviderTest {
 
@@ -144,4 +148,14 @@ public void testRetrieveFieldWithEmptyLabel() throws XmlPullParserException, IOE
         assertEquals(FormField.Type.hidden, usernameFormField.getType());
         assertEquals("", usernameFormField.getLabel());
     }
+
+    @ParameterizedTest
+    @EnumSource(SmackTestUtil.XmlPullParserKind.class)
+    public void testShouldThrowSmackParsingException(SmackTestUtil.XmlPullParserKind parserKind) {
+        String form = "<x xmlns='jabber:x:data' type='form'>"
+                    +   "<field/>"
+                    + "</x>";
+        SmackParsingException.RequiredValueMissingException exception = assertThrows(SmackParsingException.RequiredValueMissingException.class, () -> SmackTestUtil.parse(form, DataFormProvider.class, parserKind));
+        assertEquals("The data form field of unspecified type has no 'var' attribute, even though one is required as per XEP-0004 § 3.2", exception.getMessage());
+    }
 }