From e7094af9d57422e904e7e10fb50f7e23bd88afc3 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Tue, 20 Feb 2018 17:03:44 +0100 Subject: [PATCH] warn when attempting to write stanza to an unbound stream --- .../services/XmppConnectionService.java | 2 +- .../conversations/xmpp/XmppConnection.java | 48 +++++++++++-------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java index 681486f6a..85549c97a 100644 --- a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java +++ b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java @@ -3583,7 +3583,7 @@ public class XmppConnectionService extends Service { final XmppConnection connection = account.getXmppConnection(); if (connection != null) { IqPacket request = mIqGenerator.generateCreateAccountWithCaptcha(account, id, data); - connection.sendUnmodifiedIqPacket(request, connection.registrationResponseListener); + connection.sendUnmodifiedIqPacket(request, connection.registrationResponseListener,true); } } diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index cede6c572..2948d8c32 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -107,9 +107,9 @@ public class XmppConnection implements Runnable { private XmlReader tagReader; private TagWriter tagWriter = new TagWriter(); private final Features features = new Features(this); - private boolean needsBinding = true; private boolean shouldAuthenticate = true; private boolean inSmacksSession = false; + private boolean isBound = false; private Element streamFeatures; private final HashMap disco = new HashMap<>(); @@ -273,11 +273,12 @@ public class XmppConnection implements Runnable { Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": connecting"); features.encryptionEnabled = false; inSmacksSession = false; + isBound = false; this.attempt++; this.verifiedHostname = null; //will be set if user entered hostname is being used or hostname was verified with dnssec try { Socket localSocket; - shouldAuthenticate = needsBinding = !account.isOptionSet(Account.OPTION_REGISTER); + shouldAuthenticate = !account.isOptionSet(Account.OPTION_REGISTER); this.changeStatus(Account.State.CONNECTING); final boolean useTor = mXmppConnectionService.useTorToConnect() || account.isOnion(); final boolean extended = mXmppConnectionService.showExtendedConnectionOptions(); @@ -602,6 +603,7 @@ public class XmppConnection implements Runnable { tagWriter.writeStanzaAsync(r); } else if (nextTag.isStart("resumed")) { this.inSmacksSession = true; + this.isBound = true; this.tagWriter.writeStanzaAsync(new RequestPacket(smVersion)); lastPacketReceived = SystemClock.elapsedRealtime(); final Element resumed = tagReader.readElement(nextTag); @@ -878,6 +880,7 @@ public class XmppConnection implements Runnable { private void processStreamFeatures(final Tag currentTag) throws XmlPullParserException, IOException { this.streamFeatures = tagReader.readElement(currentTag); final boolean isSecure = features.encryptionEnabled || Config.ALLOW_NON_TLS_CONNECTIONS; + final boolean needsBinding = !isBound && !account.isOptionSet(Account.OPTION_REGISTER); if (this.streamFeatures.hasChild("starttls") && !features.encryptionEnabled) { sendStartTLS(); } else if (this.streamFeatures.hasChild("register") && account.isOptionSet(Account.OPTION_REGISTER)) { @@ -975,7 +978,7 @@ public class XmppConnection implements Runnable { register.query("jabber:iq:register").addChild(username); register.query().addChild(password); register.setFrom(account.getJid().toBareJid()); - sendUnmodifiedIqPacket(register, registrationResponseListener); + sendUnmodifiedIqPacket(register, registrationResponseListener, true); } else if (query.hasChild("x", Namespace.DATA)) { final Data data = Data.parse(query.findChild("x", Namespace.DATA)); final Element blob = query.findChild("data", "urn:xmpp:bob"); @@ -1025,7 +1028,7 @@ public class XmppConnection implements Runnable { throw new StateChangingError(Account.State.REGISTRATION_FAILED); } } - }); + },true); } private void setAccountCreationFailed(String url) { @@ -1065,7 +1068,6 @@ public class XmppConnection implements Runnable { Log.d(Config.LOGTAG,account.getJid().toBareJid()+": interrupted while waiting for DB restore during bind"); return; } - needsBinding = false; clearIqCallbacks(); final IqPacket iq = new IqPacket(IqPacket.TYPE.SET); iq.addChild("bind", Namespace.BIND).addChild("resource").setContent(account.getResource()); @@ -1077,6 +1079,7 @@ public class XmppConnection implements Runnable { } final Element bind = packet.findChild("bind"); if (bind != null && packet.getType() == IqPacket.TYPE.RESULT) { + isBound = true; final Element jid = bind.findChild("jid"); if (jid != null && jid.getContent() != null) { try { @@ -1114,7 +1117,7 @@ public class XmppConnection implements Runnable { } throw new StateChangingError(Account.State.BIND_FAILURE); } - }); + },true); } private void clearIqCallbacks() { @@ -1154,16 +1157,13 @@ public class XmppConnection implements Runnable { Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": sending legacy session to outdated server"); final IqPacket startSession = new IqPacket(IqPacket.TYPE.SET); startSession.addChild("session", "urn:ietf:params:xml:ns:xmpp-session"); - this.sendUnmodifiedIqPacket(startSession, new OnIqPacketReceived() { - @Override - public void onIqPacketReceived(Account account, IqPacket packet) { - if (packet.getType() == IqPacket.TYPE.RESULT) { - sendPostBindInitialization(); - } else if (packet.getType() != IqPacket.TYPE.TIMEOUT) { - throw new StateChangingError(Account.State.SESSION_FAILURE); - } + this.sendUnmodifiedIqPacket(startSession, (account, packet) -> { + if (packet.getType() == IqPacket.TYPE.RESULT) { + sendPostBindInitialization(); + } else if (packet.getType() != IqPacket.TYPE.TIMEOUT) { + throw new StateChangingError(Account.State.SESSION_FAILURE); } - }); + },true); } private void sendPostBindInitialization() { @@ -1369,10 +1369,10 @@ public class XmppConnection implements Runnable { public String sendIqPacket(final IqPacket packet, final OnIqPacketReceived callback) { packet.setFrom(account.getJid()); - return this.sendUnmodifiedIqPacket(packet, callback); + return this.sendUnmodifiedIqPacket(packet, callback, false); } - public synchronized String sendUnmodifiedIqPacket(final IqPacket packet, final OnIqPacketReceived callback) { + public synchronized String sendUnmodifiedIqPacket(final IqPacket packet, final OnIqPacketReceived callback, boolean force) { if (packet.getId() == null) { packet.setAttribute("id", nextRandomId()); } @@ -1381,7 +1381,7 @@ public class XmppConnection implements Runnable { packetCallbacks.put(packet.getId(), new Pair<>(packet, callback)); } } - this.sendPacket(packet); + this.sendPacket(packet,force); return packet.getId(); } @@ -1394,13 +1394,21 @@ public class XmppConnection implements Runnable { } private synchronized void sendPacket(final AbstractStanza packet) { + sendPacket(packet,false); + } + + private synchronized void sendPacket(final AbstractStanza packet, final boolean force) { if (stanzasSent == Integer.MAX_VALUE) { resetStreamId(); disconnect(true); return; } synchronized (this.mStanzaQueue) { - tagWriter.writeStanzaAsync(packet); + if (force || isBound) { + tagWriter.writeStanzaAsync(packet); + } else { + Log.d(Config.LOGTAG,account.getJid().toBareJid()+" do not write stanza to unbound stream "+packet.toString()); + } if (packet instanceof AbstractAcknowledgeableStanza) { AbstractAcknowledgeableStanza stanza = (AbstractAcknowledgeableStanza) packet; @@ -1413,7 +1421,7 @@ public class XmppConnection implements Runnable { ++stanzasSent; this.mStanzaQueue.append(stanzasSent, stanza); - if (stanza instanceof MessagePacket && stanza.getId() != null && getFeatures().sm()) { + if (stanza instanceof MessagePacket && stanza.getId() != null && inSmacksSession) { if (Config.EXTENDED_SM_LOGGING) { Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": requesting ack for message stanza #" + stanzasSent); }