Do not check exception message in PacketParserUtilsTest

as it may be localized.

This also makes PacketParserUtilsTest to use Junit5.

Fixes SMACK-874.

documentation/extensions/index.md

@@ -86,6 +86,7 @@ Experimental Smack Extensions and currently supported XEPs of smack-experimental
 |-----------------------------------------------------------|--------------------------------------------------------|-----------|-------------------------------------------------------------------------------------------------------------------------|
 | Message Carbons                                           | [XEP-0280](https://xmpp.org/extensions/xep-0280.html)  | n/a       | Keep all IM clients for a user engaged in a conversation, by carbon-copy outbound messages to all interested resources. |
 | [Message Archive Management](mam.md)                      | [XEP-0313](https://xmpp.org/extensions/xep-0313.html)  | n/a       | Query and control an archive of messages stored on a server. |
+| Data Forms XML Element                                    | [XEP-0315](https://xmpp.org/extensions/xep-0315.html)  | n/a       | Allows to include XML-data in XEP-0004 data forms. |
 | [Internet of Things - Sensor Data](iot.md)                | [XEP-0323](https://xmpp.org/extensions/xep-0323.html)  | n/a       | Sensor data interchange over XMPP. |
 | [Internet of Things - Provisioning](iot.md)               | [XEP-0324](https://xmpp.org/extensions/xep-0324.html)  | n/a       | Provisioning, access rights and user privileges for the Internet of Things. |
 | [Internet of Things - Control](iot.md)                    | [XEP-0325](https://xmpp.org/extensions/xep-0325.html)  | n/a       | Describes how to control devices or actuators in an XMPP-based sensor network. |

smack-core/src/main/java/org/jivesoftware/smack/SmackConfiguration.java

@@ -17,6 +17,8 @@
 
 package org.jivesoftware.smack;
 
+import java.net.MalformedURLException;
+import java.net.URL;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -50,6 +52,18 @@ import org.jivesoftware.smack.util.Objects;
  */
 public final class SmackConfiguration {
 
+    public static final String SMACK_URL_STRING = "https://igniterealtime.org/projects/smack";
+
+    public static final URL SMACK_URL;
+
+    static {
+        try {
+            SMACK_URL = new URL(SMACK_URL_STRING);
+        } catch (MalformedURLException e) {
+            throw new IllegalStateException(e);
+        }
+    }
+
     private static int defaultPacketReplyTimeout = 5000;
     private static int packetCollectorSize = 5000;
 

smack-core/src/main/java/org/jivesoftware/smack/packet/IQ.java

@@ -261,6 +261,8 @@ public abstract class IQ extends Stanza {
     }
 
     protected final void initializeAsResultFor(IQ request) {
+        assert this != request;
+
         if (!(request.getType() == Type.get || request.getType() == Type.set)) {
             throw new IllegalArgumentException(
                     "IQ must be of type 'set' or 'get'. Original IQ: " + request.toXML());

smack-core/src/main/java/org/jivesoftware/smack/util/ExceptionUtil.java

@@ -22,6 +22,10 @@ import java.io.StringWriter;
 public class ExceptionUtil {
 
     public static String getStackTrace(Throwable throwable) {
+        if (throwable == null) {
+            return null;
+        }
+
         StringWriter stringWriter = new StringWriter();
         PrintWriter printWriter = new PrintWriter(stringWriter);
 

smack-core/src/main/java/org/jivesoftware/smack/util/XmppElementUtil.java

@@ -16,13 +16,30 @@
  */
 package org.jivesoftware.smack.util;
 
+import java.util.logging.Logger;
+
 import javax.xml.namespace.QName;
 
 import org.jivesoftware.smack.packet.FullyQualifiedElement;
 
 public class XmppElementUtil {
 
+    public static final Logger LOGGER = Logger.getLogger(XmppElementUtil.class.getName());
+
     public static QName getQNameFor(Class<? extends FullyQualifiedElement> fullyQualifiedElement) {
+        try {
+            Object qnameObject = fullyQualifiedElement.getField("QNAME").get(null);
+            if (QName.class.isAssignableFrom(qnameObject.getClass())) {
+                return (QName) qnameObject;
+            }
+            LOGGER.warning("The QNAME field of " + fullyQualifiedElement + " is not of type QNAME.");
+        } catch (NoSuchFieldException e) {
+            LOGGER.finer("The class " + fullyQualifiedElement + " has no static QNAME field. Consider adding one.");
+            // Proceed to fallback strategy.
+        } catch (IllegalArgumentException | IllegalAccessException | SecurityException e) {
+            throw new IllegalArgumentException(e);
+        }
+
         String element, namespace;
         try {
             element = (String) fullyQualifiedElement.getField("ELEMENT").get(null);

smack-core/src/test/java/org/jivesoftware/smack/packet/TestIQ.java

@@ -16,10 +16,12 @@
  */
 package org.jivesoftware.smack.packet;
 
+import org.jivesoftware.smack.SmackConfiguration;
+
 public class TestIQ extends SimpleIQ {
 
     public TestIQ() {
-        this("https://igniterealtime.org/projects/smack", "test-iq");
+        this(SmackConfiguration.SMACK_URL_STRING, "test-iq");
     }
 
     public TestIQ(String element, String namespace) {

smack-core/src/test/java/org/jivesoftware/smack/util/PacketParserUtilsTest.java

@@ -22,15 +22,13 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Properties;
-import java.util.function.Supplier;
-import java.util.stream.Stream;
 
 import javax.xml.parsers.FactoryConfigurationError;
 import javax.xml.parsers.ParserConfigurationException;
@@ -50,8 +48,9 @@ import org.jivesoftware.smack.xml.XmlPullParser;
 import org.jivesoftware.smack.xml.XmlPullParserException;
 
 import com.jamesmurty.utils.XMLBuilder;
-import org.junit.Ignore;
-import org.junit.Test;
+
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.EnumSource;
 import org.xml.sax.SAXException;
@@ -428,7 +427,7 @@ public class PacketParserUtilsTest {
     }
 
     // TODO: Re-enable once we throw an exception on duplicate body elements.
-    @Ignore
+    @Disabled
     @Test
     public void duplicateMessageBodiesTest()
             throws FactoryConfigurationError, XmlPullParserException, IOException, Exception {
@@ -460,7 +459,7 @@ public class PacketParserUtilsTest {
         assertXmlNotSimilar(control, message.toXML().toString());
     }
 
-    @Ignore
+    @Disabled
     @Test
     public void duplicateMessageBodiesTest2()
             throws FactoryConfigurationError, XmlPullParserException, IOException, Exception {
@@ -679,7 +678,7 @@ public class PacketParserUtilsTest {
      *
      * @throws Exception
      */
-    @Test(expected = XmlPullParserException.class)
+    @Test
     public void invalidMessageBodyContainingTagTest() throws Exception {
         String control = XMLBuilder.create("message")
             .namespace(StreamOpen.CLIENT_NAMESPACE)
@@ -695,14 +694,14 @@ public class PacketParserUtilsTest {
                     .t("Bad Message Body")
             .asString(outputProperties);
 
-        Message message = PacketParserUtils.parseMessage(TestUtils.getMessageParser(control));
-
-        fail("Should throw exception. Instead got message: " + message.toXML().toString());
+        assertThrows(XmlPullParserException.class, () ->
+            PacketParserUtils.parseMessage(TestUtils.getMessageParser(control))
+        );
     }
 
     @Test
     public void invalidXMLInMessageBody() throws Exception {
-        String validControl = XMLBuilder.create("message")
+        final String validControl = XMLBuilder.create("message")
             .namespace(StreamOpen.CLIENT_NAMESPACE)
             .a("from", "romeo@montague.lit/orchard")
             .a("to", "juliet@capulet.lit/balcony")
@@ -713,41 +712,20 @@ public class PacketParserUtilsTest {
                 .t("Good Message Body")
             .asString(outputProperties);
 
-        // XPP3 writes "end tag", StAX writes "end-tag".
-        Supplier<Stream<String>> expectedContentOfExceptionMessage = () -> Stream.of("end tag", "end-tag");
-
-        String invalidControl = validControl.replace("Good Message Body", "Bad </span> Body");
-
-        try {
+        assertThrows(XmlPullParserException.class, () -> {
+            String invalidControl = validControl.replace("Good Message Body", "Bad </span> Body");
             PacketParserUtils.parseMessage(PacketParserUtils.getParserFor(invalidControl));
-            fail("Exception should be thrown");
-        } catch (XmlPullParserException e) {
-            String exceptionMessage = e.getMessage();
-            boolean expectedContentFound = expectedContentOfExceptionMessage.get().anyMatch((expected) -> exceptionMessage.contains(expected));
-            assertTrue(expectedContentFound);
-        }
+        });
 
-        invalidControl = validControl.replace("Good Message Body", "Bad </body> Body");
-
-        try {
+        assertThrows(XmlPullParserException.class, () -> {
+            String invalidControl = validControl.replace("Good Message Body", "Bad </body> Body");
             PacketParserUtils.parseMessage(PacketParserUtils.getParserFor(invalidControl));
-            fail("Exception should be thrown");
-        } catch (XmlPullParserException e) {
-            String exceptionMessage = e.getMessage();
-            boolean expectedContentFound = expectedContentOfExceptionMessage.get().anyMatch((expected) -> exceptionMessage.contains(expected));
-            assertTrue(expectedContentFound);
-        }
+        });
 
-        invalidControl = validControl.replace("Good Message Body", "Bad </message> Body");
-
-        try {
+        assertThrows(XmlPullParserException.class, () -> {
+            String invalidControl = validControl.replace("Good Message Body", "Bad </message> Body");
             PacketParserUtils.parseMessage(PacketParserUtils.getParserFor(invalidControl));
-            fail("Exception should be thrown");
-        } catch (XmlPullParserException e) {
-            String exceptionMessage = e.getMessage();
-            boolean expectedContentFound = expectedContentOfExceptionMessage.get().anyMatch((expected) -> exceptionMessage.contains(expected));
-            assertTrue(expectedContentFound);
-        }
+        });
     }
 
     @Test
@@ -891,15 +869,18 @@ public class PacketParserUtilsTest {
         return otherLanguage;
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void descriptiveTextNullLangPassedMap() throws Exception {
         final String text = "Dummy descriptive text";
         Map<String, String> texts = new HashMap<>();
         texts.put(null, text);
-        StanzaError
-            .getBuilder(StanzaError.Condition.internal_server_error)
-            .setDescriptiveTexts(texts)
-            .build();
+
+        assertThrows(IllegalArgumentException.class, () ->
+            StanzaError
+                .getBuilder(StanzaError.Condition.internal_server_error)
+                .setDescriptiveTexts(texts)
+                .build()
+        );
     }
 
     @Test

smack-experimental/src/main/java/org/jivesoftware/smackx/xmlelement/DataFormsXmlElementManager.java

@@ -0,0 +1,28 @@
+/**
+ *
+ * Copyright 2019 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.xmlelement;
+
+import org.jivesoftware.smackx.xdata.provider.FormFieldChildElementProviderManager;
+import org.jivesoftware.smackx.xmlelement.provider.DataFormsXmlElementProvider;
+
+public class DataFormsXmlElementManager {
+
+    static {
+        FormFieldChildElementProviderManager.addFormFieldChildElementProvider(new DataFormsXmlElementProvider());
+    }
+
+}

smack-experimental/src/main/java/org/jivesoftware/smackx/xmlelement/element/DataFormsXmlElement.java

@@ -0,0 +1,74 @@
+/**
+ *
+ * Copyright 2019 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.xmlelement.element;
+
+import javax.xml.namespace.QName;
+
+import org.jivesoftware.smack.packet.StandardExtensionElement;
+import org.jivesoftware.smack.packet.XmlEnvironment;
+import org.jivesoftware.smack.util.XmlStringBuilder;
+import org.jivesoftware.smackx.xdata.FormField;
+import org.jivesoftware.smackx.xdata.FormFieldChildElement;
+
+public class DataFormsXmlElement implements FormFieldChildElement {
+
+    public static final String ELEMENT = "wrapper";
+
+    public static final String NAMESPACE = "urn:xmpp:xml-element";
+
+    public static final QName QNAME = new QName(NAMESPACE, ELEMENT);
+
+    private final StandardExtensionElement payload;
+
+    public DataFormsXmlElement(StandardExtensionElement payload) {
+        this.payload = payload;
+    }
+
+    @Override
+    public QName getQName() {
+        return QNAME;
+    }
+
+    @Override
+    public String getElementName() {
+        return ELEMENT;
+    }
+
+    @Override
+    public String getNamespace() {
+        return NAMESPACE;
+    }
+
+    @Override
+    public XmlStringBuilder toXML(XmlEnvironment xmlEnvironment) {
+        XmlStringBuilder xml = new XmlStringBuilder(this, xmlEnvironment);
+        if (payload == null) {
+            return xml.closeEmptyElement();
+        }
+
+        xml.rightAngleBracket();
+
+        xml.append(payload.toXML());
+
+        xml.closeElement(this);
+        return xml;
+    }
+
+    public static DataFormsXmlElement from(FormField formField) {
+        return (DataFormsXmlElement) formField.getFormFieldChildElement(QNAME);
+    }
+}

smack-experimental/src/main/java/org/jivesoftware/smackx/xmlelement/element/package-info.java

@@ -0,0 +1,21 @@
+/**
+ *
+ * Copyright 2019 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.
+ */
+
+/**
+ * Element classes for XEP-0315: Data Forms XML Element.
+ */
+package org.jivesoftware.smackx.xmlelement.element;

smack-experimental/src/main/java/org/jivesoftware/smackx/xmlelement/package-info.java

@@ -0,0 +1,21 @@
+/**
+ *
+ * Copyright 2019 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.
+ */
+
+/**
+ * Smacks implementation of XEP-0315: Data Forms XML Element.
+ */
+package org.jivesoftware.smackx.xmlelement;

smack-experimental/src/main/java/org/jivesoftware/smackx/xmlelement/provider/DataFormsXmlElementProvider.java

@@ -0,0 +1,54 @@
+/**
+ *
+ * Copyright 2019 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.xmlelement.provider;
+
+import java.io.IOException;
+
+import javax.xml.namespace.QName;
+
+import org.jivesoftware.smack.packet.StandardExtensionElement;
+import org.jivesoftware.smack.packet.XmlEnvironment;
+import org.jivesoftware.smack.parsing.SmackParsingException;
+import org.jivesoftware.smack.parsing.StandardExtensionElementProvider;
+import org.jivesoftware.smack.xml.XmlPullParser;
+import org.jivesoftware.smack.xml.XmlPullParserException;
+import org.jivesoftware.smackx.xdata.provider.FormFieldChildElementProvider;
+import org.jivesoftware.smackx.xmlelement.element.DataFormsXmlElement;
+
+public class DataFormsXmlElementProvider extends FormFieldChildElementProvider<DataFormsXmlElement> {
+
+    @Override
+    public QName getQName() {
+        return DataFormsXmlElement.QNAME;
+    }
+
+    @Override
+    public DataFormsXmlElement parse(XmlPullParser parser, int initialDepth, XmlEnvironment xmlEnvironment)
+                    throws IOException, XmlPullParserException, SmackParsingException {
+        XmlPullParser.TagEvent tagEvent = parser.nextTag();
+
+        final StandardExtensionElement standardExtensionElement;
+        if (tagEvent == XmlPullParser.TagEvent.START_ELEMENT) {
+            standardExtensionElement = StandardExtensionElementProvider.INSTANCE.parse(parser);
+        } else {
+            standardExtensionElement = null;
+        }
+
+        return new DataFormsXmlElement(standardExtensionElement);
+    }
+
+}

smack-experimental/src/main/java/org/jivesoftware/smackx/xmlelement/provider/package-info.java

@@ -0,0 +1,21 @@
+/**
+ *
+ * Copyright 2019 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.
+ */
+
+/**
+ * Provider classes for XEP-0315: Data Forms XML Element.
+ */
+package org.jivesoftware.smackx.xmlelement.provider;

smack-experimental/src/main/resources/org.jivesoftware.smack.experimental/experimental.xml

@@ -7,5 +7,6 @@
         <className>org.jivesoftware.smackx.httpfileupload.HttpFileUploadManager</className>
 		<className>org.jivesoftware.smackx.eme.ExplicitMessageEncryptionManager</className>
         <className>org.jivesoftware.smackx.sid.StableUniqueStanzaIdManager</className>
+        <className>org.jivesoftware.smackx.xmlelement.DataFormsXmlElementManager</className>
     </startupClasses>
 </smack>

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

@@ -36,6 +36,7 @@ import java.util.logging.Logger;
 import org.jivesoftware.smack.AbstractConnectionListener;
 import org.jivesoftware.smack.ConnectionCreationListener;
 import org.jivesoftware.smack.Manager;
+import org.jivesoftware.smack.SmackConfiguration;
 import org.jivesoftware.smack.SmackException.NoResponseException;
 import org.jivesoftware.smack.SmackException.NotConnectedException;
 import org.jivesoftware.smack.StanzaListener;
@@ -92,7 +93,7 @@ public final class EntityCapsManager extends Manager {
      */
     private static final String DEFAULT_HASH = StringUtils.SHA1;
 
-    private static String DEFAULT_ENTITY_NODE = "http://www.igniterealtime.org/projects/smack";
+    private static String DEFAULT_ENTITY_NODE = SmackConfiguration.SMACK_URL_STRING;
 
     protected static EntityCapsPersistentCache persistentCache;
 

smack-extensions/src/main/java/org/jivesoftware/smackx/disco/packet/DiscoverInfo.java

@@ -203,7 +203,7 @@ public class DiscoverInfo extends IQ implements TypedCloneable<DiscoverInfo> {
      * @param node the node attribute that supplements the 'jid' attribute
      */
     public void setNode(String node) {
-        this.node = node;
+        this.node = StringUtils.requireNullOrNotEmpty(node, "The node can not be the empty string");
     }
 
     /**

smack-extensions/src/main/java/org/jivesoftware/smackx/iqprivate/PrivateDataManager.java

@@ -25,6 +25,7 @@ import java.util.WeakHashMap;
 import javax.xml.namespace.QName;
 
 import org.jivesoftware.smack.Manager;
+import org.jivesoftware.smack.SmackConfiguration;
 import org.jivesoftware.smack.SmackException.NoResponseException;
 import org.jivesoftware.smack.SmackException.NotConnectedException;
 import org.jivesoftware.smack.XMPPConnection;
@@ -193,7 +194,7 @@ public final class PrivateDataManager extends Manager {
 
         @Override
         public String getNamespace() {
-            return "https://igniterealtime.org/projects/smack/";
+            return SmackConfiguration.SMACK_URL_STRING;
         }
 
         @Override

smack-extensions/src/main/java/org/jivesoftware/smackx/pubsub/Node.java

@@ -46,6 +46,10 @@ import org.jivesoftware.smackx.shim.packet.Header;
 import org.jivesoftware.smackx.shim.packet.HeadersExtension;
 import org.jivesoftware.smackx.xdata.Form;
 
+import org.jxmpp.jid.Jid;
+import org.jxmpp.jid.impl.JidCreate;
+import org.jxmpp.stringprep.XmppStringprepException;
+
 public abstract class Node {
     protected final PubSubManager pubSubManager;
     protected final String id;
@@ -386,12 +390,45 @@ public abstract class Node {
      * @throws NotConnectedException
      * @throws InterruptedException
      */
-    public Subscription subscribe(String jid) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException {
+    public Subscription subscribe(Jid jid) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException {
         PubSub pubSub = createPubsubPacket(Type.set, new SubscribeExtension(jid, getId()));
         PubSub reply = sendPubsubPacket(pubSub);
         return reply.getExtension(PubSubElementType.SUBSCRIPTION);
     }
 
+    /**
+     * The user subscribes to the node using the supplied jid.  The
+     * bare jid portion of this one must match the jid for the connection.
+     *
+     * Please note that the {@link Subscription.State} should be checked
+     * on return since more actions may be required by the caller.
+     * {@link Subscription.State#pending} - The owner must approve the subscription
+     * request before messages will be received.
+     * {@link Subscription.State#unconfigured} - If the {@link Subscription#isConfigRequired()} is true,
+     * the caller must configure the subscription before messages will be received.  If it is false
+     * the caller can configure it but is not required to do so.
+     *
+     * @param jidString The jid to subscribe as.
+     * @return The subscription
+     * @throws XMPPErrorException
+     * @throws NoResponseException
+     * @throws NotConnectedException
+     * @throws InterruptedException
+     * @throws IllegalArgumentException if the provided string is not a valid JID.
+     * @deprecated use {@link #subscribe(Jid)} instead.
+     */
+    @Deprecated
+    // TODO: Remove in Smack 4.5.
+    public Subscription subscribe(String jidString) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException {
+        Jid jid;
+        try {
+            jid = JidCreate.from(jidString);
+        } catch (XmppStringprepException e) {
+            throw new IllegalArgumentException(e);
+        }
+        return subscribe(jid);
+    }
+
     /**
      * The user subscribes to the node using the supplied jid and subscription
      * options.  The bare jid portion of this one must match the jid for the
@@ -414,13 +451,49 @@ public abstract class Node {
      * @throws NotConnectedException
      * @throws InterruptedException
      */
-    public Subscription subscribe(String jid, SubscribeForm subForm) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException {
+    public Subscription subscribe(Jid jid, SubscribeForm subForm) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException {
         PubSub request = createPubsubPacket(Type.set, new SubscribeExtension(jid, getId()));
         request.addExtension(new FormNode(FormNodeType.OPTIONS, subForm));
         PubSub reply = sendPubsubPacket(request);
         return reply.getExtension(PubSubElementType.SUBSCRIPTION);
     }
 
+    /**
+     * The user subscribes to the node using the supplied jid and subscription
+     * options.  The bare jid portion of this one must match the jid for the
+     * connection.
+     *
+     * Please note that the {@link Subscription.State} should be checked
+     * on return since more actions may be required by the caller.
+     * {@link Subscription.State#pending} - The owner must approve the subscription
+     * request before messages will be received.
+     * {@link Subscription.State#unconfigured} - If the {@link Subscription#isConfigRequired()} is true,
+     * the caller must configure the subscription before messages will be received.  If it is false
+     * the caller can configure it but is not required to do so.
+     *
+     * @param jidString The jid to subscribe as.
+     * @param subForm
+     *
+     * @return The subscription
+     * @throws XMPPErrorException
+     * @throws NoResponseException
+     * @throws NotConnectedException
+     * @throws InterruptedException
+     * @throws IllegalArgumentException if the provided string is not a valid JID.
+     * @deprecated use {@link #subscribe(Jid, SubscribeForm)} instead.
+     */
+    @Deprecated
+    // TODO: Remove in Smack 4.5.
+    public Subscription subscribe(String jidString, SubscribeForm subForm) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException {
+        Jid jid;
+        try {
+            jid = JidCreate.from(jidString);
+        } catch (XmppStringprepException e) {
+            throw new IllegalArgumentException(e);
+        }
+        return subscribe(jid, subForm);
+    }
+
     /**
      * Remove the subscription related to the specified JID.  This will only
      * work if there is only 1 subscription.  If there are multiple subscriptions,

smack-extensions/src/main/java/org/jivesoftware/smackx/pubsub/PubSubManager.java

@@ -37,6 +37,7 @@ import org.jivesoftware.smack.packet.IQ.Type;
 import org.jivesoftware.smack.packet.Stanza;
 import org.jivesoftware.smack.packet.StanzaError;
 import org.jivesoftware.smack.packet.StanzaError.Condition;
+import org.jivesoftware.smack.util.StringUtils;
 
 import org.jivesoftware.smackx.disco.ServiceDiscoveryManager;
 import org.jivesoftware.smackx.disco.packet.DiscoverInfo;
@@ -268,6 +269,7 @@ public final class PubSubManager extends Manager {
      * @throws NotAPubSubNodeException
      */
     public Node getNode(String id) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException, NotAPubSubNodeException {
+        StringUtils.requireNotNullNorEmpty(id, "The node ID can not be null or the empty string");
         Node node = nodeMap.get(id);
 
         if (node == null) {

smack-extensions/src/main/java/org/jivesoftware/smackx/pubsub/SubscribeExtension.java

@@ -16,42 +16,39 @@
  */
 package org.jivesoftware.smackx.pubsub;
 
+import org.jivesoftware.smack.packet.XmlEnvironment;
+import org.jivesoftware.smack.util.XmlStringBuilder;
+
+import org.jxmpp.jid.Jid;
+
 /**
  * Represents a request to subscribe to a node.
  *
  * @author Robin Collier
  */
 public class SubscribeExtension extends NodeExtension {
-    protected String jid;
+    protected final Jid jid;
 
-    public SubscribeExtension(String subscribeJid) {
+    public SubscribeExtension(Jid subscribeJid) {
         super(PubSubElementType.SUBSCRIBE);
         jid = subscribeJid;
     }
 
-    public SubscribeExtension(String subscribeJid, String nodeId) {
+    public SubscribeExtension(Jid subscribeJid, String nodeId) {
         super(PubSubElementType.SUBSCRIBE, nodeId);
         jid = subscribeJid;
     }
 
-    public String getJid() {
+    public Jid getJid() {
         return jid;
     }
 
     @Override
-    public String toXML(org.jivesoftware.smack.packet.XmlEnvironment enclosingNamespace) {
-        StringBuilder builder = new StringBuilder("<");
-        builder.append(getElementName());
-
-        if (getNode() != null) {
-            builder.append(" node='");
-            builder.append(getNode());
-            builder.append('\'');
-        }
-        builder.append(" jid='");
-        builder.append(getJid());
-        builder.append("'/>");
-
-        return builder.toString();
+    public XmlStringBuilder toXML(XmlEnvironment xmlEnvironment) {
+        XmlStringBuilder xml = new XmlStringBuilder(this, xmlEnvironment);
+        xml.optAttribute("node", getNode());
+        xml.attribute("jid", getJid());
+        xml.closeEmptyElement();
+        return xml;
     }
 }

smack-extensions/src/test/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5ByteStreamManagerTest.java

@@ -28,7 +28,6 @@ import static org.mockito.Mockito.mock;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.net.ConnectException;
 import java.net.InetAddress;
 import java.net.ServerSocket;
 import java.util.List;
@@ -521,17 +520,24 @@ public class Socks5ByteStreamManagerTest {
      * @throws InterruptedException
      * @throws SmackException
      * @throws XMPPException
+     * @throws XmppStringprepException
      */
     @Test
     public void shouldFailIfInitiatorCannotConnectToSocks5Proxy()
-                    throws SmackException, InterruptedException, XMPPException {
+                    throws SmackException, InterruptedException, XMPPException, XmppStringprepException {
         final Protocol protocol = new Protocol();
         final XMPPConnection connection = ConnectionUtils.createMockedConnection(protocol, initiatorJID);
         final String sessionID = "session_id_shouldFailIfInitiatorCannotConnectToSocks5Proxy";
 
+        // TODO: The following two variables should be named initatorProxyJid and initiatorProxyAddress.
+        final DomainBareJid proxyJID = JidCreate.domainBareFrom("s5b-proxy.initiator.org");
+        // Use an TEST-NET-1 address from RFC 5737 to act as black hole.
+        final String proxyAddress = "192.0.2.1";
+
         // get Socks5ByteStreamManager for connection
         Socks5BytestreamManager byteStreamManager = Socks5BytestreamManager.getBytestreamManager(connection);
         byteStreamManager.setAnnounceLocalStreamHost(false);
+        byteStreamManager.setProxyConnectionTimeout(3000);
 
         /**
          * create responses in the order they should be queried specified by the XEP-0065
@@ -602,8 +608,9 @@ public class Socks5ByteStreamManagerTest {
 
         // initiator can't connect to proxy because it is not running
         protocol.verifyAll();
-        Throwable actualCause = e.getCause().getCause();
-        assertEquals("Unexpected throwable: " + actualCause + '.' + ExceptionUtil.getStackTrace(actualCause), ConnectException.class, actualCause.getClass());
+        Throwable actualCause = e.getCause();
+        assertEquals("Unexpected throwable: " + actualCause + '.' + ExceptionUtil.getStackTrace(actualCause),
+                        TimeoutException.class, actualCause.getClass());
     }
 
     /**

smack-integration-test/build.gradle

@@ -23,6 +23,8 @@ dependencies {
 	// (ab)uses @Before from org.junit
 	compile "org.junit.vintage:junit-vintage-engine:$junitVersion"
 	compile 'junit:junit:4.12'
+	// Add Junit 5 API for e.g. assertThrows()
+	implementation "org.junit.jupiter:junit-jupiter-api:$junitVersion"
 	testCompile "org.jxmpp:jxmpp-jid:$jxmppVersion:tests"
 }
 

smack-integration-test/src/main/java/org/jivesoftware/smackx/pubsub/PubSubIntegrationTest.java

@@ -18,14 +18,13 @@ package org.jivesoftware.smackx.pubsub;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.util.List;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import org.jivesoftware.smack.SmackConfiguration;
 import org.jivesoftware.smack.SmackException.NoResponseException;
 import org.jivesoftware.smack.SmackException.NotConnectedException;
 import org.jivesoftware.smack.XMPPException.XMPPErrorException;
+import org.jivesoftware.smack.packet.StandardExtensionElement;
 import org.jivesoftware.smack.packet.StanzaError;
 
 import org.igniterealtime.smack.inttest.AbstractSmackIntegrationTest;
@@ -69,8 +68,6 @@ public class PubSubIntegrationTest extends AbstractSmackIntegrationTest {
         try {
             LeafNode leafNode = (LeafNode) node;
             leafNode.publish();
-            List<Item> items = leafNode.getItems();
-            assertTrue(items.isEmpty());
         }
         finally {
             pubSubManagerOne.deleteNode(nodename);
@@ -95,6 +92,7 @@ public class PubSubIntegrationTest extends AbstractSmackIntegrationTest {
     public void transientNotificationOnlyNodeWithItemTest() throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException {
         final String nodename = "sinttest-transient-notificationonly-withitem-nodename-" + testRunId;
         final String itemId = "sinttest-transient-notificationonly-withitem-itemid-" + testRunId;
+
         ConfigureForm defaultConfiguration = pubSubManagerOne.getDefaultConfiguration();
         ConfigureForm config = new ConfigureForm(defaultConfiguration.createAnswerForm());
         // Configure the node as "Notification-Only Node".
@@ -102,12 +100,23 @@ public class PubSubIntegrationTest extends AbstractSmackIntegrationTest {
         // Configure the node as "transient" (set persistent_items to 'false')
         config.setPersistentItems(false);
         Node node = pubSubManagerOne.createNode(nodename, config);
+
+        // Add a dummy payload. If there is no payload, but just an item ID, then ejabberd will *not* return an error,
+        // which I believe to be non-compliant behavior (although, granted, the XEP is not very clear about this). A user
+        // which sends an empty item with ID to an node that is configured to be notification-only and transient probably
+        // does something wrong, as the item's ID will never appear anywhere. Hence it would be nice if the user would be
+        // made aware of this issue by returning an error. Sadly ejabberd does not do so.
+        // See also https://github.com/processone/ejabberd/issues/2864#issuecomment-500741915
+        final StandardExtensionElement dummyPayload = StandardExtensionElement.builder("dummy-payload",
+                        SmackConfiguration.SMACK_URL_STRING).setText(testRunId).build();
+
         try {
-            LeafNode leafNode = (LeafNode) node;
-            leafNode.publish(new Item(itemId));
-            fail("An exception should have been thrown.");
-        }
-        catch (XMPPErrorException e) {
+            XMPPErrorException e = assertThrows(XMPPErrorException.class, () -> {
+                LeafNode leafNode = (LeafNode) node;
+
+                Item item = new PayloadItem<>(itemId, dummyPayload);
+                leafNode.publish(item);
+            });
             assertEquals(StanzaError.Type.MODIFY, e.getStanzaError().getType());
             assertNotNull(e.getStanzaError().getExtension("item-forbidden", "http://jabber.org/protocol/pubsub#errors"));
         }

smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XmppNioTcpConnection.java

@@ -1116,7 +1116,9 @@ public class XmppNioTcpConnection extends AbstractXmppNioConnection {
             // remote hostname information, in which case peerHost needs to be specified." that A should be used. TLS
             // session resumption may would need or at least benefit from B. Variant A would also be required if the
             // String is used for certificate verification. And it appears at least likely that TLS session resumption
-            // would not be hurt by using variant A. Therfore we currently use variant A.
+            // would not be hurt by using variant A. Therefore we currently use variant A.
+            // TODO: Should we use the ACE representation of the XMPP service domain? Compare with f60e4055ec529f0b8160acedf13275592ab10a4b
+            // If yes, then we should probably introduce getXmppServiceDomainAceEncodedIfPossible().
             engine = smackTlsContext.sslContext.createSSLEngine(config.getXMPPServiceDomain().toString(), remoteAddress.getPort());
             engine.setUseClientMode(true);