diff --git a/resources/releasedocs/changelog.html b/resources/releasedocs/changelog.html index 3d872c85f..06c247f84 100644 --- a/resources/releasedocs/changelog.html +++ b/resources/releasedocs/changelog.html @@ -141,7 +141,33 @@ hr {
+

4.4.1 -- 2021-03-03

+

Bug +

+ + +

Improvement +

+

4.4.0 -- 2020-12-06

diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/XmlUtil.java b/smack-core/src/main/java/org/jivesoftware/smack/util/XmlUtil.java index b55297d6e..3845da548 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/XmlUtil.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/XmlUtil.java @@ -64,4 +64,13 @@ public class XmlUtil { return stringWriter.toString(); } + + public static boolean isClarkNotation(String text) { + if (text.isEmpty()) { + return false; + } + + // TODO: This is currently a mediocre heuristic to check for clark notation. + return text.charAt(0) == '{'; + } } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java index 67cf93555..885448fda 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java @@ -360,8 +360,7 @@ public final class FileTransferNegotiator extends Manager { } private static DataForm createDefaultInitiationForm() { - DataForm.Builder form = DataForm.builder() - .setType(DataForm.Type.form); + DataForm.Builder form = DataForm.builder(DataForm.Type.form); ListSingleFormField.Builder fieldBuilder = FormField.listSingleBuilder(STREAM_DATA_FIELD_NAME); if (!IBB_ONLY) { 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 index 4941d236d..97156974f 100644 --- 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. @@ -18,27 +18,26 @@ package org.jivesoftware.smackx.formtypes; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.logging.Logger; import org.jivesoftware.smack.util.Objects; - +import org.jivesoftware.smack.util.StringUtils; +import org.jivesoftware.smack.util.XmlUtil; import org.jivesoftware.smackx.xdata.FormField; import org.jivesoftware.smackx.xdata.TextSingleFormField; import org.jivesoftware.smackx.xdata.packet.DataForm; public class FormFieldRegistry { + private static final Logger LOGGER = Logger.getLogger(FormFieldRegistry.class.getName()); + private static final Map> REGISTRY = new HashMap<>(); - private static final Map LOOKASIDE_REGISTRY = new HashMap<>(); - - private static final Map FIELD_NAME_TO_FORM_TYPE = new HashMap<>(); - - static { - register(FormField.FORM_TYPE, FormField.Type.hidden); - } + private static final Map CLARK_NOTATION_FIELD_REGISTRY = new ConcurrentHashMap<>(); @SuppressWarnings("ReferenceEquality") - public static synchronized void register(DataForm dataForm) { + public static void register(DataForm dataForm) { // TODO: Also allow forms of type 'result'? if (dataForm.getType() != DataForm.Type.form) { throw new IllegalArgumentException(); @@ -56,64 +55,57 @@ public class FormFieldRegistry { continue; } - String fieldName = formField.getFieldName(); FormField.Type type = formField.getType(); + if (type == FormField.Type.fixed) { + continue; + } + + String fieldName = formField.getFieldName(); register(formType, fieldName, type); } } - public static synchronized void register(String formType, String fieldName, FormField.Type type) { + public static void register(String formType, String fieldName, FormField.Type fieldType) { + StringUtils.requireNotNullNorEmpty(fieldName, "fieldName must be provided"); + Objects.requireNonNull(fieldType); + 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); + if (XmlUtil.isClarkNotation(fieldName)) { + CLARK_NOTATION_FIELD_REGISTRY.put(fieldName, fieldType); } - - LOOKASIDE_REGISTRY.put(fieldName, type); return; } - Map fieldNameToType = REGISTRY.get(formType); - if (fieldNameToType == null) { - fieldNameToType = new HashMap<>(); - REGISTRY.put(formType, fieldNameToType); - } else { - FormField.Type previousType = fieldNameToType.get(fieldName); - if (previousType != null && previousType != type) { - throw new IllegalArgumentException(); + FormField.Type previousType; + synchronized (REGISTRY) { + Map fieldNameToType = REGISTRY.get(formType); + if (fieldNameToType == null) { + fieldNameToType = new HashMap<>(); + REGISTRY.put(formType, fieldNameToType); + } else { + previousType = fieldNameToType.get(fieldName); + if (previousType != null && previousType != fieldType) { + throw new IllegalArgumentException(); + } } + previousType = fieldNameToType.put(fieldName, fieldType); } - fieldNameToType.put(fieldName, type); - - 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 + "'"); + if (previousType != null && fieldType != previousType) { + LOGGER.warning("Form field registry inconsitency detected: Registered field '" + fieldName + "' of type " + fieldType + " but previous type was " + previousType); } - LOOKASIDE_REGISTRY.put(fieldName, type); } - public static synchronized FormField.Type lookup(String formType, String fieldName) { - if (formType != null) { + public static FormField.Type lookup(String formType, String fieldName) { + if (formType == null) { + if (!XmlUtil.isClarkNotation(fieldName)) { + return null; + } + + return CLARK_NOTATION_FIELD_REGISTRY.get(fieldName); + } + + synchronized (REGISTRY) { Map fieldNameToTypeMap = REGISTRY.get(formType); if (fieldNameToTypeMap != null) { FormField.Type type = fieldNameToTypeMap.get(fieldName); @@ -121,38 +113,13 @@ public class FormFieldRegistry { return type; } } - } else { - formType = FIELD_NAME_TO_FORM_TYPE.get(fieldName); - if (formType != null) { - FormField.Type type = lookup(formType, fieldName); - if (type != null) { - return type; - } - } } - // Fallback to lookaside registry. - return LOOKASIDE_REGISTRY.get(fieldName); + return null; } - public static synchronized FormFieldInformation lookup(String fieldName) { - String formType = FIELD_NAME_TO_FORM_TYPE.get(fieldName); - FormField.Type type = lookup(formType, fieldName); - if (type == null) { - return null; - } - - return new FormFieldInformation(type, formType); + public static synchronized FormField.Type lookup(String fieldName) { + return lookup(null, fieldName); } - public static final class FormFieldInformation { - public final FormField.Type formFieldType; - public final String formType; - - - private FormFieldInformation(FormField.Type formFieldType, String formType) { - this.formFieldType = formFieldType; - this.formType = formType; - } - } } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/form/FillableForm.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/form/FillableForm.java index a9c6aa008..9954e8a2e 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/form/FillableForm.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/form/FillableForm.java @@ -255,11 +255,16 @@ public class FillableForm extends FilledForm { if (!missingRequiredFields.isEmpty()) { throw new IllegalStateException("Not all required fields filled. Missing: " + missingRequiredFields); } - DataForm dataFormToSend = DataForm.builder() - .addField(formTypeFormField) - .addFields(filledFields.values()) - .build(); - return dataFormToSend; + DataForm.Builder builder = DataForm.builder(); + + // the submit form has the same FORM_TYPE as the form. + if (formTypeFormField != null) { + builder.addField(formTypeFormField); + } + + builder.addFields(filledFields.values()); + + return builder.build(); } } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/form/FilledForm.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/form/FilledForm.java index e541eadbc..8727832f1 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/form/FilledForm.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/form/FilledForm.java @@ -17,7 +17,6 @@ package org.jivesoftware.smackx.xdata.form; import org.jivesoftware.smack.util.Objects; -import org.jivesoftware.smack.util.StringUtils; import org.jivesoftware.smackx.xdata.FormField; import org.jivesoftware.smackx.xdata.TextSingleFormField; @@ -31,10 +30,6 @@ public abstract class FilledForm implements FormReader { public FilledForm(DataForm dataForm) { this.dataForm = Objects.requireNonNull(dataForm); - String formType = dataForm.getFormType(); - if (StringUtils.isNullOrEmpty(formType)) { - throw new IllegalArgumentException("The provided data form has no hidden FROM_TYPE field."); - } if (dataForm.getType() == DataForm.Type.cancel) { throw new IllegalArgumentException("Forms of type 'cancel' are not filled nor fillable"); } 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 index 0457e69f3..9ab354d57 100644 --- 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2003-2007 Jive Software, 2020 Florian Schmaus. + * Copyright 2003-2007 Jive Software, 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. @@ -100,6 +100,9 @@ public final class DataForm implements ExtensionElement { instructions = CollectionUtil.cloneAndSeal(builder.instructions); reportedData = builder.reportedData; items = CollectionUtil.cloneAndSeal(builder.items); + + builder.orderFields(); + fields = CollectionUtil.cloneAndSeal(builder.fields); fieldsMap = CollectionUtil.cloneAndSeal(builder.fieldsMap); extensionElements = CollectionUtil.cloneAndSeal(builder.extensionElements); @@ -353,6 +356,7 @@ public final class DataForm implements ExtensionElement { } public static final class Builder { + // TODO: Make this field final once setType() is gone. private Type type; private String title; private List instructions; @@ -381,6 +385,39 @@ public final class DataForm implements ExtensionElement { extensionElements = CollectionUtil.newListWith(dataForm.getExtensionElements()); } + private void orderFields() { + Iterator it = fields.iterator(); + if (!it.hasNext()) { + return; + } + + FormField hiddenFormTypeField = it.next().asHiddenFormTypeFieldIfPossible(); + if (hiddenFormTypeField != null) { + // The hidden FROM_TYPE field is already in first position, nothing to do. + return; + } + + while (it.hasNext()) { + hiddenFormTypeField = it.next().asHiddenFormTypeFieldIfPossible(); + if (hiddenFormTypeField != null) { + // Remove the hidden FORM_TYPE field that is not on first position. + it.remove(); + // And insert it again at first position. + fields.add(0, hiddenFormTypeField); + break; + } + } + } + + /** + * Deprecated do not use. + * + * @param type the type. + * @return a reference to this builder. + * @deprecated use {@link DataForm#builder(Type)} instead. + */ + @Deprecated + // TODO: Remove in Smack 4.5 and then make this.type final. public Builder setType(Type type) { this.type = Objects.requireNonNull(type); return this; @@ -538,6 +575,8 @@ public final class DataForm implements ExtensionElement { private final List fields; + private Map fieldMap; + public ReportedData(List fields) { this.fields = Collections.unmodifiableList(fields); } @@ -561,6 +600,18 @@ public final class DataForm implements ExtensionElement { return fields; } + public FormField getField(String name) { + if (fieldMap == null) { + fieldMap = new HashMap<>(fields.size()); + for (FormField field : fields) { + String fieldName = field.getFieldName(); + fieldMap.put(fieldName, field); + } + } + + return fieldMap.get(name); + } + @Override public XmlStringBuilder toXML(XmlEnvironment xmlEnvironment) { XmlStringBuilder xml = new XmlStringBuilder(this, xmlEnvironment); 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 index a126d1092..6465efacb 100644 --- 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2003-2007 Jive Software 2020 Florian Schmaus. + * Copyright 2003-2007 Jive Software 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. @@ -66,10 +66,10 @@ public class DataFormProvider extends ExtensionElementProvider { @Override public DataForm parse(XmlPullParser parser, int initialDepth, XmlEnvironment xmlEnvironment) throws XmlPullParserException, IOException, SmackParsingException { DataForm.Type dataFormType = DataForm.Type.fromString(parser.getAttributeValue("", "type")); - DataForm.Builder dataForm = DataForm.builder(); - dataForm.setType(dataFormType); + DataForm.Builder dataForm = DataForm.builder(dataFormType); String formType = null; + DataForm.ReportedData reportedData = null; outerloop: while (true) { XmlPullParser.Event eventType = parser.next(); @@ -86,6 +86,8 @@ public class DataFormProvider extends ExtensionElementProvider { dataForm.setTitle(parser.nextText()); break; case "field": + // Note that we parse this form field without any potential reportedData. We only use reportedData + // to lookup form field types of fields under . FormField formField = parseField(parser, elementXmlEnvironment, formType); TextSingleFormField hiddenFormTypeField = formField.asHiddenFormTypeFieldIfPossible(); @@ -99,12 +101,15 @@ public class DataFormProvider extends ExtensionElementProvider { dataForm.addField(formField); break; case "item": - DataForm.Item item = parseItem(parser, elementXmlEnvironment, formType); + DataForm.Item item = parseItem(parser, elementXmlEnvironment, formType, reportedData); dataForm.addItem(item); break; case "reported": - DataForm.ReportedData reported = parseReported(parser, elementXmlEnvironment, formType); - dataForm.setReportedData(reported); + if (reportedData != null) { + throw new SmackParsingException("Data form with multiple elements"); + } + reportedData = parseReported(parser, elementXmlEnvironment, formType); + dataForm.setReportedData(reportedData); break; // See XEP-133 Example 32 for a corner case where the data form contains this extension. case RosterPacket.ELEMENT: @@ -135,6 +140,11 @@ public class DataFormProvider extends ExtensionElementProvider { private static FormField parseField(XmlPullParser parser, XmlEnvironment xmlEnvironment, String formType) throws XmlPullParserException, IOException, SmackParsingException { + return parseField(parser, xmlEnvironment, formType, null); + } + + private static FormField parseField(XmlPullParser parser, XmlEnvironment xmlEnvironment, String formType, DataForm.ReportedData reportedData) + throws XmlPullParserException, IOException, SmackParsingException { final int initialDepth = parser.getDepth(); final String fieldName = parser.getAttributeValue("var"); @@ -186,15 +196,30 @@ public class DataFormProvider extends ExtensionElementProvider { } } + // Data forms of type 'result' may contain and elements. If this is the case, then the type + // of the s within the elements is determined by the information found in . See + // XEP-0004 § 3.4 and SMACK-902 + if (type == null && reportedData != null) { + FormField reportedFormField = reportedData.getField(fieldName); + if (reportedFormField != null) { + type = reportedFormField.getType(); + } + } + 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; + } } } @@ -301,7 +326,8 @@ public class DataFormProvider extends ExtensionElementProvider { return builder; } - private static DataForm.Item parseItem(XmlPullParser parser, XmlEnvironment xmlEnvironment, String formType) + private static DataForm.Item parseItem(XmlPullParser parser, XmlEnvironment xmlEnvironment, String formType, + DataForm.ReportedData reportedData) throws XmlPullParserException, IOException, SmackParsingException { final int initialDepth = parser.getDepth(); List fields = new ArrayList<>(); @@ -312,7 +338,8 @@ public class DataFormProvider extends ExtensionElementProvider { String name = parser.getName(); switch (name) { case "field": - FormField field = parseField(parser, XmlEnvironment.from(parser, xmlEnvironment), formType); + FormField field = parseField(parser, XmlEnvironment.from(parser, xmlEnvironment), formType, + reportedData); fields.add(field); break; } diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/xdata/packet/DataFormTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/xdata/packet/DataFormTest.java index bb225316c..7fe6e2b0c 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/smackx/xdata/packet/DataFormTest.java +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/xdata/packet/DataFormTest.java @@ -152,4 +152,14 @@ public class DataFormTest extends SmackTestSuite { DataForm df = pr.parse(PacketParserUtils.getParserFor(formWithFixedField)); assertEquals(FormField.Type.fixed, df.getFields().get(0).getType()); } + + @Test + public void testReorderHiddenFormTypeFieldAtFirstPosition() { + DataForm dataForm = DataForm.builder() + .addField(FormField.textSingleBuilder("foo1").setValue("bar").build()) + .addField(FormField.textSingleBuilder("foo2").setValue("baz").build()) + .setFormType("my-form-type") + .build(); + assertNotNull(dataForm.getFields().get(0).asHiddenFormTypeFieldIfPossible()); + } } diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/xdata/provider/DataFormProviderTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/xdata/provider/DataFormProviderTest.java new file mode 100644 index 000000000..f25eb67d3 --- /dev/null +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/xdata/provider/DataFormProviderTest.java @@ -0,0 +1,117 @@ +/** + * + * Copyright 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.smackx.xdata.provider; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.IOException; +import java.util.List; + +import org.jivesoftware.smack.parsing.SmackParsingException; +import org.jivesoftware.smack.util.PacketParserUtils; +import org.jivesoftware.smack.xml.XmlPullParser; +import org.jivesoftware.smack.xml.XmlPullParserException; +import org.jivesoftware.smackx.xdata.FormField; +import org.jivesoftware.smackx.xdata.packet.DataForm; + +import org.junit.jupiter.api.Test; + +public class DataFormProviderTest { + + @Test + public void testRetrieveFieldTypeFromReported() throws XmlPullParserException, IOException, SmackParsingException { + + String firstForm = + "" + + " Advanced User Search" + + " The following fields are available for searching. Wildcard (*) characters are allowed as part of the query." + + " " + + " jabber:iq:search" + + " " + + " " + + " " + + " " + + " " + + " true" + + " " + + " " + + " true" + + " " + + " " + + " true" + + " " + + ""; + XmlPullParser parser = PacketParserUtils.getParserFor(firstForm); + DataForm firstDataForm = DataFormProvider.INSTANCE.parse(parser); + FormField usernameFormField = firstDataForm.getField("Username"); + assertEquals(FormField.Type.bool, usernameFormField.getType()); + + String secondForm = + "" + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " 0" + + " " + + " " + + " " + + " frank@orphu" + + " " + + " " + + " " + + " frank" + + " " + + " " + + " " + + " " + + " 0" + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + " frank2@orphu" + + " " + + " " + + " " + + " frank2" + + " " + + " " + + " " + + " " + + " " + + " " + + " " + + ""; + parser = PacketParserUtils.getParserFor(secondForm); + DataForm secondDataForm = DataFormProvider.INSTANCE.parse(parser); + List items = secondDataForm.getItems(); + assertEquals(2, items.size()); + } + +}