From 322463bab739e38e87df13f6a5766276e24ddefe Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sat, 28 Apr 2018 16:26:40 +0200 Subject: [PATCH 1/2] return InvalidJid object instead of null if Jid can not be parsed --- .../conversations/entities/Bookmark.java | 3 +- .../conversations/parser/AbstractParser.java | 7 +- .../siacs/conversations/parser/IqParser.java | 7 +- .../conversations/parser/MessageParser.java | 25 +++- .../conversations/parser/PresenceParser.java | 3 +- .../eu/siacs/conversations/xml/Element.java | 3 +- .../siacs/conversations/xmpp/InvalidJid.java | 140 ++++++++++++++++++ .../conversations/xmpp/XmppConnection.java | 94 ++++++------ .../xmpp/jingle/JingleCandidate.java | 3 +- .../AbstractAcknowledgeableStanza.java | 6 + .../xmpp/stanzas/AbstractStanza.java | 21 +-- .../conversations/xmpp/stanzas/IqPacket.java | 6 + 12 files changed, 249 insertions(+), 69 deletions(-) create mode 100644 src/main/java/eu/siacs/conversations/xmpp/InvalidJid.java diff --git a/src/main/java/eu/siacs/conversations/entities/Bookmark.java b/src/main/java/eu/siacs/conversations/entities/Bookmark.java index b1fa55119..1157d823a 100644 --- a/src/main/java/eu/siacs/conversations/entities/Bookmark.java +++ b/src/main/java/eu/siacs/conversations/entities/Bookmark.java @@ -11,6 +11,7 @@ import java.util.Locale; import eu.siacs.conversations.utils.UIHelper; import eu.siacs.conversations.xml.Element; +import eu.siacs.conversations.xmpp.InvalidJid; import rocks.xmpp.addr.Jid; public class Bookmark extends Element implements ListItem { @@ -35,7 +36,7 @@ public class Bookmark extends Element implements ListItem { Bookmark bookmark = new Bookmark(account); bookmark.setAttributes(element.getAttributes()); bookmark.setChildren(element.getChildren()); - bookmark.jid = bookmark.getAttributeAsJid("jid"); + bookmark.jid = InvalidJid.getNullForInvalid(bookmark.getAttributeAsJid("jid")); return bookmark; } diff --git a/src/main/java/eu/siacs/conversations/parser/AbstractParser.java b/src/main/java/eu/siacs/conversations/parser/AbstractParser.java index abe5d8f19..05297d74d 100644 --- a/src/main/java/eu/siacs/conversations/parser/AbstractParser.java +++ b/src/main/java/eu/siacs/conversations/parser/AbstractParser.java @@ -11,6 +11,7 @@ import eu.siacs.conversations.entities.Conversation; import eu.siacs.conversations.entities.MucOptions; import eu.siacs.conversations.services.XmppConnectionService; import eu.siacs.conversations.xml.Element; +import eu.siacs.conversations.xmpp.InvalidJid; import eu.siacs.conversations.xmpp.stanzas.AbstractStanza; import rocks.xmpp.addr.Jid; @@ -37,7 +38,7 @@ public abstract class AbstractParser { } for(Element child : element.getChildren()) { if ("delay".equals(child.getName()) && "urn:xmpp:delay".equals(child.getNamespace())) { - final Jid f = to == null ? null : child.getAttributeAsJid("from"); + final Jid f = to == null ? null : InvalidJid.getNullForInvalid(child.getAttributeAsJid("from")); if (f != null && (to.asBareJid().equals(f) || to.getDomain().equals(f.toString()))) { continue; } @@ -115,7 +116,9 @@ public abstract class AbstractParser { } Jid realJid = item.getAttributeAsJid("jid"); MucOptions.User user = new MucOptions.User(conference.getMucOptions(), fullJid); - user.setRealJid(realJid); + if (InvalidJid.isValid(realJid)) { + user.setRealJid(realJid); + } user.setAffiliation(affiliation); user.setRole(role); return user; diff --git a/src/main/java/eu/siacs/conversations/parser/IqParser.java b/src/main/java/eu/siacs/conversations/parser/IqParser.java index 9c4efc01e..d41aa170b 100644 --- a/src/main/java/eu/siacs/conversations/parser/IqParser.java +++ b/src/main/java/eu/siacs/conversations/parser/IqParser.java @@ -29,6 +29,7 @@ import eu.siacs.conversations.entities.Contact; import eu.siacs.conversations.services.XmppConnectionService; import eu.siacs.conversations.xml.Namespace; import eu.siacs.conversations.xml.Element; +import eu.siacs.conversations.xmpp.InvalidJid; import eu.siacs.conversations.xmpp.OnIqPacketReceived; import eu.siacs.conversations.xmpp.OnUpdateBlocklist; import eu.siacs.conversations.xmpp.stanzas.IqPacket; @@ -47,7 +48,7 @@ public class IqParser extends AbstractParser implements OnIqPacketReceived { } for (final Element item : query.getChildren()) { if (item.getName().equals("item")) { - final Jid jid = item.getAttributeAsJid("jid"); + final Jid jid = InvalidJid.getNullForInvalid(item.getAttributeAsJid("jid")); if (jid == null) { continue; } @@ -310,7 +311,7 @@ public class IqParser extends AbstractParser implements OnIqPacketReceived { // Create a collection of Jids from the packet for (final Element item : items) { if (item.getName().equals("item")) { - final Jid jid = item.getAttributeAsJid("jid"); + final Jid jid = InvalidJid.getNullForInvalid(item.getAttributeAsJid("jid")); if (jid != null) { jids.add(jid); } @@ -344,7 +345,7 @@ public class IqParser extends AbstractParser implements OnIqPacketReceived { final Collection jids = new ArrayList<>(items.size()); for (final Element item : items) { if (item.getName().equals("item")) { - final Jid jid = item.getAttributeAsJid("jid"); + final Jid jid = InvalidJid.getNullForInvalid(item.getAttributeAsJid("jid")); if (jid != null) { jids.add(jid); } diff --git a/src/main/java/eu/siacs/conversations/parser/MessageParser.java b/src/main/java/eu/siacs/conversations/parser/MessageParser.java index c84d401e1..6da26fd11 100644 --- a/src/main/java/eu/siacs/conversations/parser/MessageParser.java +++ b/src/main/java/eu/siacs/conversations/parser/MessageParser.java @@ -32,6 +32,7 @@ import eu.siacs.conversations.services.XmppConnectionService; import eu.siacs.conversations.utils.CryptoHelper; import eu.siacs.conversations.xml.Namespace; import eu.siacs.conversations.xml.Element; +import eu.siacs.conversations.xmpp.InvalidJid; import eu.siacs.conversations.xmpp.OnMessagePacketReceived; import eu.siacs.conversations.xmpp.chatstate.ChatState; import eu.siacs.conversations.xmpp.pep.Avatar; @@ -64,7 +65,7 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece for (Element child : packet.getChildren()) { if (child.getName().equals("stanza-id") && Namespace.STANZA_IDS.equals(child.getNamespace()) - && by.equals(child.getAttributeAsJid("by"))) { + && by.equals(InvalidJid.getNullForInvalid(child.getAttributeAsJid("by")))) { return child.getAttribute("id"); } } @@ -73,7 +74,7 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece private static Jid getTrueCounterpart(Element mucUserElement, Jid fallback) { final Element item = mucUserElement == null ? null : mucUserElement.findChild("item"); - Jid result = item == null ? null : item.getAttributeAsJid("jid"); + Jid result = item == null ? null : InvalidJid.getNullForInvalid(item.getAttributeAsJid("jid")); return result != null ? result : fallback; } @@ -139,17 +140,25 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece if (x != null) { Element invite = x.findChild("invite"); if (invite != null) { - Element pw = x.findChild("password"); - Jid from = invite.getAttributeAsJid("from"); + String password = x.findChildContent("password"); + Jid from = InvalidJid.getNullForInvalid(invite.getAttributeAsJid("from")); Contact contact = from == null ? null : account.getRoster().getContact(from); - return new Invite(message.getAttributeAsJid("from"), pw != null ? pw.getContent() : null, contact); + Jid room = InvalidJid.getNullForInvalid(message.getAttributeAsJid("from")); + if (room == null) { + return null; + } + return new Invite(room, password, contact); } } else { x = message.findChild("x", "jabber:x:conference"); if (x != null) { - Jid from = message.getAttributeAsJid("from"); + Jid from = InvalidJid.getNullForInvalid(message.getAttributeAsJid("from")); Contact contact = from == null ? null : account.getRoster().getContact(from); - return new Invite(x.getAttributeAsJid("jid"), x.getAttribute("password"), contact); + Jid room = InvalidJid.getNullForInvalid(x.getAttributeAsJid("jid")); + if (room == null) { + return null; + } + return new Invite(room, x.getAttribute("password"), contact); } } return null; @@ -693,7 +702,7 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece Element displayed = packet.findChild("displayed", "urn:xmpp:chat-markers:0"); if (displayed != null) { final String id = displayed.getAttribute("id"); - final Jid sender = displayed.getAttributeAsJid("sender"); + final Jid sender = InvalidJid.getNullForInvalid(displayed.getAttributeAsJid("sender")); if (packet.fromAccount(account) && !selfAddressed) { dismissNotification(account, counterpart, query); } else if (isTypeGroupChat) { diff --git a/src/main/java/eu/siacs/conversations/parser/PresenceParser.java b/src/main/java/eu/siacs/conversations/parser/PresenceParser.java index 3f828a71e..610d40e9e 100644 --- a/src/main/java/eu/siacs/conversations/parser/PresenceParser.java +++ b/src/main/java/eu/siacs/conversations/parser/PresenceParser.java @@ -22,6 +22,7 @@ import eu.siacs.conversations.generator.PresenceGenerator; import eu.siacs.conversations.services.XmppConnectionService; import eu.siacs.conversations.xml.Element; import eu.siacs.conversations.xml.Namespace; +import eu.siacs.conversations.xmpp.InvalidJid; import eu.siacs.conversations.xmpp.OnPresencePacketReceived; import eu.siacs.conversations.xmpp.pep.Avatar; import eu.siacs.conversations.xmpp.stanzas.PresencePacket; @@ -69,7 +70,7 @@ public class PresenceParser extends AbstractParser implements if (item != null && !from.isBareJid()) { mucOptions.setError(MucOptions.Error.NONE); MucOptions.User user = parseItem(conversation, item, from); - if (codes.contains(MucOptions.STATUS_CODE_SELF_PRESENCE) || (codes.contains(MucOptions.STATUS_CODE_ROOM_CREATED) && jid.equals(item.getAttributeAsJid("jid")))) { + if (codes.contains(MucOptions.STATUS_CODE_SELF_PRESENCE) || (codes.contains(MucOptions.STATUS_CODE_ROOM_CREATED) && jid.equals(InvalidJid.getNullForInvalid(item.getAttributeAsJid("jid"))))) { if (mucOptions.setOnline()) { mXmppConnectionService.getAvatarService().clear(mucOptions); } diff --git a/src/main/java/eu/siacs/conversations/xml/Element.java b/src/main/java/eu/siacs/conversations/xml/Element.java index 94ff6c564..b103e8647 100644 --- a/src/main/java/eu/siacs/conversations/xml/Element.java +++ b/src/main/java/eu/siacs/conversations/xml/Element.java @@ -11,6 +11,7 @@ import java.util.Locale; import eu.siacs.conversations.Config; import eu.siacs.conversations.utils.XmlHelper; +import eu.siacs.conversations.xmpp.InvalidJid; import rocks.xmpp.addr.Jid; public class Element { @@ -153,7 +154,7 @@ public class Element { try { return Jid.ofEscaped(jid); } catch (final IllegalArgumentException e) { - return null; + return new InvalidJid(jid); } } return null; diff --git a/src/main/java/eu/siacs/conversations/xmpp/InvalidJid.java b/src/main/java/eu/siacs/conversations/xmpp/InvalidJid.java new file mode 100644 index 000000000..e1f257fd3 --- /dev/null +++ b/src/main/java/eu/siacs/conversations/xmpp/InvalidJid.java @@ -0,0 +1,140 @@ +/* + * Copyright (c) 2018, Daniel Gultsch All rights reserved. + * + * Redistribution and use in source and binary forms, with or without modification, + * are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation and/or + * other materials provided with the distribution. + * + * 3. Neither the name of the copyright holder nor the names of its contributors + * may be used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR + * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package eu.siacs.conversations.xmpp; + +import android.support.annotation.NonNull; + +import rocks.xmpp.addr.Jid; + +public class InvalidJid implements Jid { + + private final String value; + + public InvalidJid(String jid) { + this.value = jid; + } + + @Override + @NonNull + public String toString() { + return value; + } + + @Override + public boolean isFullJid() { + throw new AssertionError("Not implemented"); + } + + @Override + public boolean isBareJid() { + throw new AssertionError("Not implemented"); + } + + @Override + public Jid asBareJid() { + throw new AssertionError("Not implemented"); + } + + @Override + public Jid withLocal(CharSequence charSequence) { + throw new AssertionError("Not implemented"); + } + + @Override + public Jid withResource(CharSequence charSequence) { + throw new AssertionError("Not implemented"); + } + + @Override + public Jid atSubdomain(CharSequence charSequence) { + throw new AssertionError("Not implemented"); + } + + @Override + public String getLocal() { + throw new AssertionError("Not implemented"); + } + + @Override + public String getEscapedLocal() { + throw new AssertionError("Not implemented"); + } + + @Override + public String getDomain() { + throw new AssertionError("Not implemented"); + } + + @Override + public String getResource() { + throw new AssertionError("Not implemented"); + } + + @Override + public String toEscapedString() { + throw new AssertionError("Not implemented"); + } + + @Override + public int length() { + return value.length(); + } + + @Override + public char charAt(int index) { + return value.charAt(index); + } + + @Override + public CharSequence subSequence(int start, int end) { + return value.subSequence(start, end); + } + + @Override + public int compareTo(@NonNull Jid o) { + throw new AssertionError("Not implemented"); + } + + public static Jid getNullForInvalid(Jid jid) { + if (jid != null && jid instanceof InvalidJid) { + return null; + } else { + return jid; + } + } + + public static boolean isValid(Jid jid) { + if (jid != null && jid instanceof InvalidJid) { + return false; + } else { + return true; + } + } +} diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 3cd2e79be..72452a310 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -5,6 +5,7 @@ import android.graphics.Bitmap; import android.graphics.BitmapFactory; import android.os.SystemClock; import android.security.KeyChain; +import android.support.annotation.NonNull; import android.util.Base64; import android.util.Log; import android.util.Pair; @@ -179,6 +180,25 @@ public class XmppConnection implements Runnable { mXmppConnectionService = service; } + private static void fixResource(Context context, Account account) { + String resource = account.getResource(); + int fixedPartLength = context.getString(R.string.app_name).length() + 1; //include the trailing dot + int randomPartLength = 4; // 3 bytes + if (resource != null && resource.length() > fixedPartLength + randomPartLength) { + if (validBase64(resource.substring(fixedPartLength, fixedPartLength + randomPartLength))) { + account.setResource(resource.substring(0, fixedPartLength + randomPartLength)); + } + } + } + + private static boolean validBase64(String input) { + try { + return Base64.decode(input, Base64.URL_SAFE).length == 3; + } catch (Throwable throwable) { + return false; + } + } + protected void changeStatus(final Account.State nextStatus) { synchronized (this) { if (Thread.currentThread().isInterrupted()) { @@ -643,7 +663,7 @@ public class XmppConnection implements Runnable { private void acknowledgeStanzaUpTo(int serverCount) { if (serverCount > stanzasSent) { - Log.e(Config.LOGTAG,"server acknowledged more stanzas than we sent. serverCount="+serverCount+", ourCount="+stanzasSent); + Log.e(Config.LOGTAG, "server acknowledged more stanzas than we sent. serverCount=" + serverCount + ", ourCount=" + stanzasSent); } for (int i = 0; i < mStanzaQueue.size(); ++i) { if (serverCount >= mStanzaQueue.keyAt(i)) { @@ -661,8 +681,8 @@ public class XmppConnection implements Runnable { } } - private Element processPacket(final Tag currentTag, final int packetType) - throws XmlPullParserException, IOException { + private @NonNull + Element processPacket(final Tag currentTag, final int packetType) throws XmlPullParserException, IOException { Element element; switch (packetType) { case PACKET_IQ: @@ -675,7 +695,7 @@ public class XmppConnection implements Runnable { element = new PresencePacket(); break; default: - return null; + throw new AssertionError("Should never encounter invalid type"); } element.setAttributes(currentTag.getAttributes()); Tag nextTag = tagReader.readTag(); @@ -707,7 +727,7 @@ public class XmppConnection implements Runnable { if (inSmacksSession) { ++stanzasReceived; } else if (features.sm()) { - Log.d(Config.LOGTAG,account.getJid().asBareJid()+": not counting stanza("+element.getClass().getSimpleName()+"). Not in smacks session."); + Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": not counting stanza(" + element.getClass().getSimpleName() + "). Not in smacks session."); } lastPacketReceived = SystemClock.elapsedRealtime(); if (Config.BACKGROUND_STANZA_LOGGING && mXmppConnectionService.checkListeners()) { @@ -718,11 +738,10 @@ public class XmppConnection implements Runnable { private void processIq(final Tag currentTag) throws XmlPullParserException, IOException { final IqPacket packet = (IqPacket) processPacket(currentTag, PACKET_IQ); - - if (packet.getId() == null) { - return; // an iq packet without id is definitely invalid + if (!packet.valid()) { + Log.e(Config.LOGTAG, "encountered invalid iq from='" + packet.getFrom() + "' to='" + packet.getTo() + "'"); + return; } - if (packet instanceof JinglePacket) { if (this.jingleListener != null) { this.jingleListener.onJinglePacketReceived(account, (JinglePacket) packet); @@ -764,11 +783,19 @@ public class XmppConnection implements Runnable { private void processMessage(final Tag currentTag) throws XmlPullParserException, IOException { final MessagePacket packet = (MessagePacket) processPacket(currentTag, PACKET_MESSAGE); + if (!packet.valid()) { + Log.e(Config.LOGTAG, "encountered invalid message from='" + packet.getFrom() + "' to='" + packet.getTo() + "'"); + return; + } this.messageListener.onMessagePacketReceived(account, packet); } private void processPresence(final Tag currentTag) throws XmlPullParserException, IOException { PresencePacket packet = (PresencePacket) processPacket(currentTag, PACKET_PRESENCE); + if (!packet.valid()) { + Log.e(Config.LOGTAG, "encountered invalid presence from='" + packet.getFrom() + "' to='" + packet.getTo() + "'"); + return; + } this.presenceListener.onPresencePacketReceived(account, packet); } @@ -951,7 +978,7 @@ public class XmppConnection implements Runnable { } } throw new StateChangingError(Account.State.REGISTRATION_FAILED); - } else if (query.hasChild("instructions") || query.hasChild("x",Namespace.OOB)) { + } else if (query.hasChild("instructions") || query.hasChild("x", Namespace.OOB)) { final String instructions = query.findChildContent("instructions"); final Element oob = query.findChild("x", Namespace.OOB); final String url = oob == null ? null : oob.findChildContent("url"); @@ -965,7 +992,7 @@ public class XmppConnection implements Runnable { } throw new StateChangingError(Account.State.REGISTRATION_FAILED); } - },true); + }, true); } private void setAccountCreationFailed(String url) { @@ -1002,7 +1029,7 @@ public class XmppConnection implements Runnable { try { mXmppConnectionService.restoredFromDatabaseLatch.await(); } catch (InterruptedException e) { - Log.d(Config.LOGTAG,account.getJid().asBareJid()+": interrupted while waiting for DB restore during bind"); + Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": interrupted while waiting for DB restore during bind"); return; } clearIqCallbacks(); @@ -1026,7 +1053,7 @@ public class XmppConnection implements Runnable { try { Jid assignedJid = Jid.ofEscaped(jid.getContent()); if (!account.getJid().getDomain().equals(assignedJid.getDomain())) { - Log.d(Config.LOGTAG,account.getJid().asBareJid()+": server tried to re-assign domain to "+assignedJid.getDomain()); + Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": server tried to re-assign domain to " + assignedJid.getDomain()); throw new StateChangingError(Account.State.BIND_FAILURE); } if (account.setJid(assignedJid)) { @@ -1054,7 +1081,7 @@ public class XmppConnection implements Runnable { account.setResource(createNewResource()); } throw new StateChangingError(Account.State.BIND_FAILURE); - },true); + }, true); } private void clearIqCallbacks() { @@ -1100,7 +1127,7 @@ public class XmppConnection implements Runnable { } else if (packet.getType() != IqPacket.TYPE.TIMEOUT) { throw new StateChangingError(Account.State.SESSION_FAILURE); } - },true); + }, true); } private void sendPostBindInitialization() { @@ -1225,7 +1252,7 @@ public class XmppConnection implements Runnable { final List elements = packet.query().getChildren(); for (final Element element : elements) { if (element.getName().equals("item")) { - final Jid jid = element.getAttributeAsJid("jid"); + final Jid jid = InvalidJid.getNullForInvalid(element.getAttributeAsJid("jid")); if (jid != null && !jid.equals(Jid.of(account.getServer()))) { items.add(jid); } @@ -1295,26 +1322,7 @@ public class XmppConnection implements Runnable { } private String createNewResource() { - return mXmppConnectionService.getString(R.string.app_name)+'.'+nextRandomId(true); - } - - private static void fixResource(Context context, Account account) { - String resource = account.getResource(); - int fixedPartLength = context.getString(R.string.app_name).length() + 1; //include the trailing dot - int randomPartLength = 4; // 3 bytes - if (resource != null && resource.length() > fixedPartLength + randomPartLength) { - if (validBase64(resource.substring(fixedPartLength,fixedPartLength + randomPartLength))) { - account.setResource(resource.substring(0,fixedPartLength + randomPartLength)); - } - } - } - - private static boolean validBase64(String input) { - try { - return Base64.decode(input, Base64.URL_SAFE).length == 3; - } catch (Throwable throwable) { - return false; - } + return mXmppConnectionService.getString(R.string.app_name) + '.' + nextRandomId(true); } private String nextRandomId() { @@ -1339,7 +1347,7 @@ public class XmppConnection implements Runnable { packetCallbacks.put(packet.getId(), new Pair<>(packet, callback)); } } - this.sendPacket(packet,force); + this.sendPacket(packet, force); return packet.getId(); } @@ -1352,7 +1360,7 @@ public class XmppConnection implements Runnable { } private synchronized void sendPacket(final AbstractStanza packet) { - sendPacket(packet,false); + sendPacket(packet, false); } private synchronized void sendPacket(final AbstractStanza packet, final boolean force) { @@ -1365,7 +1373,7 @@ public class XmppConnection implements Runnable { if (force || isBound) { tagWriter.writeStanzaAsync(packet); } else { - Log.d(Config.LOGTAG,account.getJid().asBareJid()+" do not write stanza to unbound stream "+packet.toString()); + Log.d(Config.LOGTAG, account.getJid().asBareJid() + " do not write stanza to unbound stream " + packet.toString()); } if (packet instanceof AbstractAcknowledgeableStanza) { AbstractAcknowledgeableStanza stanza = (AbstractAcknowledgeableStanza) packet; @@ -1465,18 +1473,18 @@ public class XmppConnection implements Runnable { final Socket currentSocket = this.socket; final CountDownLatch streamCountDownLatch = this.mStreamCountDownLatch; try { - currentTagWriter.await(1,TimeUnit.SECONDS); + currentTagWriter.await(1, TimeUnit.SECONDS); Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": closing stream"); currentTagWriter.writeTag(Tag.end("stream:stream")); if (streamCountDownLatch != null) { - if (streamCountDownLatch.await(1, TimeUnit.SECONDS)) { + if (streamCountDownLatch.await(1, TimeUnit.SECONDS)) { Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": remote ended stream"); } else { Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": remote has not closed socket. force closing"); } } } catch (InterruptedException e) { - Log.d(Config.LOGTAG,account.getJid().asBareJid()+": interrupted while gracefully closing stream"); + Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": interrupted while gracefully closing stream"); } catch (final IOException e) { Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": io exception during disconnect (" + e.getMessage() + ")"); } finally { @@ -1769,7 +1777,7 @@ public class XmppConnection implements Runnable { } public boolean pepPublishOptions() { - return hasDiscoFeature(account.getJid().asBareJid(),Namespace.PUBSUB_PUBLISH_OPTIONS); + return hasDiscoFeature(account.getJid().asBareJid(), Namespace.PUBSUB_PUBLISH_OPTIONS); } public boolean pepOmemoWhitelisted() { diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleCandidate.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleCandidate.java index bb7b60a3d..64db37f66 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleCandidate.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleCandidate.java @@ -4,6 +4,7 @@ import java.util.ArrayList; import java.util.List; import eu.siacs.conversations.xml.Element; +import eu.siacs.conversations.xmpp.InvalidJid; import rocks.xmpp.addr.Jid; public class JingleCandidate { @@ -108,7 +109,7 @@ public class JingleCandidate { JingleCandidate parsedCandidate = new JingleCandidate( candidate.getAttribute("cid"), false); parsedCandidate.setHost(candidate.getAttribute("host")); - parsedCandidate.setJid(candidate.getAttributeAsJid("jid")); + parsedCandidate.setJid(InvalidJid.getNullForInvalid(candidate.getAttributeAsJid("jid"))); parsedCandidate.setType(candidate.getAttribute("type")); parsedCandidate.setPriority(Integer.parseInt(candidate .getAttribute("priority"))); diff --git a/src/main/java/eu/siacs/conversations/xmpp/stanzas/AbstractAcknowledgeableStanza.java b/src/main/java/eu/siacs/conversations/xmpp/stanzas/AbstractAcknowledgeableStanza.java index fa5e6fbdd..095075616 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/stanzas/AbstractAcknowledgeableStanza.java +++ b/src/main/java/eu/siacs/conversations/xmpp/stanzas/AbstractAcknowledgeableStanza.java @@ -1,6 +1,8 @@ package eu.siacs.conversations.xmpp.stanzas; import eu.siacs.conversations.xml.Element; +import eu.siacs.conversations.xmpp.InvalidJid; +import rocks.xmpp.addr.Jid; abstract public class AbstractAcknowledgeableStanza extends AbstractStanza { @@ -28,4 +30,8 @@ abstract public class AbstractAcknowledgeableStanza extends AbstractStanza { } return null; } + + public boolean valid() { + return InvalidJid.isValid(getFrom()) && InvalidJid.isValid(getTo()); + } } diff --git a/src/main/java/eu/siacs/conversations/xmpp/stanzas/AbstractStanza.java b/src/main/java/eu/siacs/conversations/xmpp/stanzas/AbstractStanza.java index d17eb3254..7be738f3a 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/stanzas/AbstractStanza.java +++ b/src/main/java/eu/siacs/conversations/xmpp/stanzas/AbstractStanza.java @@ -31,20 +31,23 @@ public class AbstractStanza extends Element { } public boolean fromServer(final Account account) { - return getFrom() == null - || getFrom().equals(Jid.of(account.getServer())) - || getFrom().equals(account.getJid().asBareJid()) - || getFrom().equals(account.getJid()); + final Jid from = getFrom(); + return from == null + || from.equals(Jid.of(account.getServer())) + || from.equals(account.getJid().asBareJid()) + || from.equals(account.getJid()); } public boolean toServer(final Account account) { - return getTo() == null - || getTo().equals(Jid.of(account.getServer())) - || getTo().equals(account.getJid().asBareJid()) - || getTo().equals(account.getJid()); + final Jid to = getTo(); + return to == null + || to.equals(Jid.of(account.getServer())) + || to.equals(account.getJid().asBareJid()) + || to.equals(account.getJid()); } public boolean fromAccount(final Account account) { - return getFrom() != null && getFrom().asBareJid().equals(account.getJid().asBareJid()); + final Jid from = getFrom(); + return from != null && from.asBareJid().equals(account.getJid().asBareJid()); } } diff --git a/src/main/java/eu/siacs/conversations/xmpp/stanzas/IqPacket.java b/src/main/java/eu/siacs/conversations/xmpp/stanzas/IqPacket.java index 302dc78e8..ba8588e1f 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/stanzas/IqPacket.java +++ b/src/main/java/eu/siacs/conversations/xmpp/stanzas/IqPacket.java @@ -66,4 +66,10 @@ public class IqPacket extends AbstractAcknowledgeableStanza { return packet; } + @Override + public boolean valid() { + String id = getId(); + return id != null && super.valid(); + } + } From b1b7cf5c0aea25add7066ab5dbee70d4631dec9a Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sat, 28 Apr 2018 16:26:50 +0200 Subject: [PATCH 2/2] version bump to 2.1.3 + changelog --- CHANGELOG.md | 3 +++ build.gradle | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ef52329f..f21c2d140 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +### Version 2.1.3 +* Do not process stanzas with invalid JIDs + ### Version 2.1.2 * Fixed avatars not being displayed on new installs diff --git a/build.gradle b/build.gradle index 7c04ec884..27a2a5b51 100644 --- a/build.gradle +++ b/build.gradle @@ -65,8 +65,8 @@ android { defaultConfig { minSdkVersion 19 targetSdkVersion 25 - versionCode 266 - versionName "2.1.2" + versionCode 267 + versionName "2.1.3" archivesBaseName += "-$versionName" applicationId "eu.siacs.conversations" resValue "string", "applicationId", applicationId