From 699145ee5fdc4fb18140fb2e385c6d8fe60f81d1 Mon Sep 17 00:00:00 2001 From: Ingo Bauersachs Date: Wed, 2 Aug 2017 01:54:29 +0200 Subject: [PATCH] Get descriptive text from error without lang A stanza with Some text, i.e. without a xml:lang attribute, did not return the value 'Some text' in getDescriptiveText(). Insert the empty string as key when the attribute is not present while parsing the xml. When writing, omit the attribute if the key is the empty string. --- .../smack/packet/AbstractError.java | 14 +++++- .../smack/util/PacketParserUtils.java | 6 +++ .../smack/util/XmlStringBuilder.java | 2 +- .../smack/util/PacketParserUtilsTest.java | 43 +++++++++++++++++++ 4 files changed, 63 insertions(+), 2 deletions(-) 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()); + } }