diff --git a/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractError.java b/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractError.java index d04c41ad3..fbb68eb52 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractError.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/packet/AbstractError.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import org.jivesoftware.smack.util.Objects; import org.jivesoftware.smack.util.PacketUtil; import org.jivesoftware.smack.util.XmlStringBuilder; @@ -85,6 +86,7 @@ public class AbstractError { * @return the descriptive text or null. */ public String getDescriptiveText(String xmllang) { + Objects.requireNonNull(xmllang, "xmllang must not be null"); return descriptiveTexts.get(xmllang); } @@ -105,7 +107,8 @@ public class AbstractError { String xmllang = entry.getKey(); String text = entry.getValue(); xml.halfOpenElement("text").xmlnsAttribute(textNamespace) - .xmllangAttribute(xmllang).rightAngleBracket(); + .optXmlLangAttribute(xmllang) + .rightAngleBracket(); xml.escape(text); xml.closeElement("text"); } @@ -120,6 +123,15 @@ public class AbstractError { protected List extensions; public B setDescriptiveTexts(Map descriptiveTexts) { + if (descriptiveTexts == null) { + this.descriptiveTexts = null; + return getThis(); + } + for (String key : descriptiveTexts.keySet()) { + if (key == null) { + throw new IllegalArgumentException("descriptiveTexts cannot contain null key"); + } + } if (this.descriptiveTexts == null) { this.descriptiveTexts = descriptiveTexts; } diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java b/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java index 02a6c96a3..4c02ee080 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java @@ -749,6 +749,12 @@ public class PacketParserUtils { descriptiveTexts = new HashMap(); } String xmllang = getLanguageAttribute(parser); + if (xmllang == null) { + // XMPPError assumes the default locale, 'en', or the empty string. + // Establish the invariant that there is never null as a key. + xmllang = ""; + } + String text = parser.nextText(); String previousValue = descriptiveTexts.put(xmllang, text); assert (previousValue == null); diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java b/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java index 2579bf522..e0b4a3847 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/XmlStringBuilder.java @@ -361,7 +361,7 @@ public class XmlStringBuilder implements Appendable, CharSequence { } public XmlStringBuilder optXmlLangAttribute(String lang) { - if (lang != null) { + if (!StringUtils.isNullOrEmpty(lang)) { xmllangAttribute(lang); } return this; diff --git a/smack-core/src/test/java/org/jivesoftware/smack/util/PacketParserUtilsTest.java b/smack-core/src/test/java/org/jivesoftware/smack/util/PacketParserUtilsTest.java index 780e0bb23..bdbeb4e1a 100644 --- a/smack-core/src/test/java/org/jivesoftware/smack/util/PacketParserUtilsTest.java +++ b/smack-core/src/test/java/org/jivesoftware/smack/util/PacketParserUtilsTest.java @@ -25,7 +25,9 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.IOException; +import java.util.HashMap; import java.util.Locale; +import java.util.Map; import java.util.Properties; import javax.xml.parsers.FactoryConfigurationError; @@ -35,6 +37,7 @@ import javax.xml.transform.TransformerException; import org.jivesoftware.smack.packet.Message; import org.jivesoftware.smack.packet.Presence; import org.jivesoftware.smack.packet.Stanza; +import org.jivesoftware.smack.packet.XMPPError; import org.jivesoftware.smack.sasl.SASLError; import org.jivesoftware.smack.sasl.packet.SaslStreamElements; import org.jivesoftware.smack.sasl.packet.SaslStreamElements.SASLFailure; @@ -867,4 +870,44 @@ public class PacketParserUtilsTest { return otherLanguage; } + @Test(expected = IllegalArgumentException.class) + public void descriptiveTextNullLangPassedMap() throws Exception { + final String text = "Dummy descriptive text"; + Map texts = new HashMap<>(); + texts.put(null, text); + XMPPError + .getBuilder(XMPPError.Condition.internal_server_error) + .setDescriptiveTexts(texts) + .build(); + } + + @Test + public void ensureNoEmptyLangInDescriptiveText() throws Exception { + final String text = "Dummy descriptive text"; + Map texts = new HashMap<>(); + texts.put("", text); + XMPPError error = XMPPError + .getBuilder(XMPPError.Condition.internal_server_error) + .setDescriptiveTexts(texts) + .build(); + final String errorXml = XMLBuilder + .create(XMPPError.ERROR).a("type", "cancel").up() + .element("internal-server-error", XMPPError.NAMESPACE).up() + .element("text", XMPPError.NAMESPACE).t(text).up() + .asString(); + XmlUnitUtils.assertSimilar(errorXml, error.toXML()); + } + + @Test + public void ensureNoNullLangInParsedDescriptiveTexts() throws Exception { + final String text = "Dummy descriptive text"; + final String errorXml = XMLBuilder + .create(XMPPError.ERROR).a("type", "cancel").up() + .element("internal-server-error", XMPPError.NAMESPACE).up() + .element("text", XMPPError.NAMESPACE).t(text).up() + .asString(); + XmlPullParser parser = TestUtils.getParser(errorXml); + XMPPError error = PacketParserUtils.parseError(parser).build(); + assertEquals(text, error.getDescriptiveText()); + } }