From d1273532ce01625bf7f73885144b55ad830546fb Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 13 Dec 2021 21:15:30 +0100 Subject: [PATCH 1/3] [muc] Correctly processes self-presences The change in 52a49769f9a8 ("[muc] Check for self-presence first in presence listener") caused all self-presences (MUC user status 110) to be ignored in the further processing chain eventually invoking checkRoleModifications() and checkAffiliationModifications(). However, some servers (e.g., ejabberd) include 110 not only in the initial presence but in all following, and we ant to process these. Fixes SMACK-918 Fixes: 52a49769f9a825a8c0252efcad0d96635fb257a6 --- .../java/org/jivesoftware/smackx/muc/MultiUserChat.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java index 78d725508..92e8ec3f3 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java @@ -211,13 +211,15 @@ public class MultiUserChat { switch (presence.getType()) { case available: - Presence oldPresence = occupantsMap.put(from, presence); if (mucUser.getStatus().contains(MUCUser.Status.PRESENCE_TO_SELF_110)) { processedReflectedSelfPresence = true; synchronized (this) { notify(); } - } else if (oldPresence != null) { + } + + Presence oldPresence = occupantsMap.put(from, presence); + if (oldPresence != null) { // Get the previous occupant's affiliation & role MUCUser mucExtension = MUCUser.from(oldPresence); MUCAffiliation oldAffiliation = mucExtension.getItem().getAffiliation(); From e39adff40fd3feae41e793994f870112f57cf131 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 13 Dec 2021 21:20:40 +0100 Subject: [PATCH 2/3] [muc] Only notify() about processed self-presence once Since notify() is a rather expensive operation, we should only invoke it once. Especially since some servers include 110 in all self presences, not just the initially reflected one on MUC join. --- .../main/java/org/jivesoftware/smackx/muc/MultiUserChat.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java index 92e8ec3f3..540e419fe 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java @@ -211,7 +211,8 @@ public class MultiUserChat { switch (presence.getType()) { case available: - if (mucUser.getStatus().contains(MUCUser.Status.PRESENCE_TO_SELF_110)) { + if (!processedReflectedSelfPresence + && mucUser.getStatus().contains(MUCUser.Status.PRESENCE_TO_SELF_110)) { processedReflectedSelfPresence = true; synchronized (this) { notify(); From fc7fc10c6967926c29df8aceea3ea0079c3356b3 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 13 Dec 2021 21:49:15 +0100 Subject: [PATCH 3/3] [pubsub] Allow for character data before 's payload Fixes SMACK-918. --- .../smackx/pubsub/provider/ItemProvider.java | 35 +++++++------- .../pubsub/provider/ItemProviderTest.java | 48 +++++++++++++++++++ 2 files changed, 66 insertions(+), 17 deletions(-) create mode 100644 smack-extensions/src/test/java/org/jivesoftware/smackx/pubsub/provider/ItemProviderTest.java diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/pubsub/provider/ItemProvider.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/pubsub/provider/ItemProvider.java index 4c7d0e7ce..5842fda91 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/pubsub/provider/ItemProvider.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/pubsub/provider/ItemProvider.java @@ -50,24 +50,25 @@ public class ItemProvider extends ExtensionElementProvider { String xmlns = parser.getNamespace(); ItemNamespace itemNamespace = ItemNamespace.fromXmlns(xmlns); - XmlPullParser.Event tag = parser.next(); + XmlPullParser.TagEvent event = parser.nextTag(); + switch (event) { + case START_ELEMENT: + String payloadElemName = parser.getName(); + String payloadNS = parser.getNamespace(); - if (tag == XmlPullParser.Event.END_ELEMENT) { - return new Item(itemNamespace, id, node); - } - else { - String payloadElemName = parser.getName(); - String payloadNS = parser.getNamespace(); - - final ExtensionElementProvider extensionProvider = ProviderManager.getExtensionProvider(payloadElemName, payloadNS); - if (extensionProvider == null) { - // TODO: Should we use StandardExtensionElement in this case? And probably remove SimplePayload all together. - CharSequence payloadText = PacketParserUtils.parseElement(parser, true); - return new PayloadItem<>(itemNamespace, id, node, new SimplePayload(payloadText.toString())); - } - else { - return new PayloadItem<>(itemNamespace, id, node, extensionProvider.parse(parser)); - } + final ExtensionElementProvider extensionProvider = ProviderManager.getExtensionProvider(payloadElemName, payloadNS); + if (extensionProvider == null) { + // TODO: Should we use StandardExtensionElement in this case? And probably remove SimplePayload all together. + CharSequence payloadText = PacketParserUtils.parseElement(parser, true); + return new PayloadItem<>(itemNamespace, id, node, new SimplePayload(payloadText.toString())); + } + else { + return new PayloadItem<>(itemNamespace, id, node, extensionProvider.parse(parser)); + } + case END_ELEMENT: + return new Item(itemNamespace, id, node); + default: + throw new AssertionError("unknown: " + event); } } diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/pubsub/provider/ItemProviderTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/pubsub/provider/ItemProviderTest.java new file mode 100644 index 000000000..e7348e920 --- /dev/null +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/pubsub/provider/ItemProviderTest.java @@ -0,0 +1,48 @@ +/** + * + * 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.pubsub.provider; + +import java.io.IOException; + +import org.jivesoftware.smack.parsing.SmackParsingException; +import org.jivesoftware.smack.test.util.SmackTestUtil; +import org.jivesoftware.smack.xml.XmlPullParserException; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +public class ItemProviderTest { + + /** + * Check that {@link ItemProvider} is able to parse items which have whitespace before their Payload. + * + * @param parserKind the used parser Kind + * @throws XmlPullParserException if an XML pull parser exception occurs. + * @throws IOException if an IO exception occurs. + * @throws SmackParsingException if an Smack parsing exception occurs. + * @see SMACK-918 + */ + @ParameterizedTest + @EnumSource(SmackTestUtil.XmlPullParserKind.class) + public void whitespaceBeforeItemPayload(SmackTestUtil.XmlPullParserKind parserKind) throws XmlPullParserException, IOException, SmackParsingException { + String item = "" + + "\n " + + ""; + SmackTestUtil.parse(item, ItemProvider.class, parserKind); + } + +}