From e1cb72b9ef4d4bfb9d11fff68e296462405d6992 Mon Sep 17 00:00:00 2001 From: Sam Whited Date: Mon, 22 Dec 2014 10:50:29 -0500 Subject: Fix NPEs in blocking code --- src/main/java/eu/siacs/conversations/ui/ConversationActivity.java | 4 +++- src/main/java/eu/siacs/conversations/ui/EditAccountActivity.java | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/ui/ConversationActivity.java b/src/main/java/eu/siacs/conversations/ui/ConversationActivity.java index 3a3f0778d..c41a18c6b 100644 --- a/src/main/java/eu/siacs/conversations/ui/ConversationActivity.java +++ b/src/main/java/eu/siacs/conversations/ui/ConversationActivity.java @@ -31,6 +31,7 @@ import java.util.ArrayList; import java.util.List; import eu.siacs.conversations.R; +import eu.siacs.conversations.entities.Account; import eu.siacs.conversations.entities.Blockable; import eu.siacs.conversations.entities.Contact; import eu.siacs.conversations.entities.Conversation; @@ -316,7 +317,8 @@ public class ConversationActivity extends XmppActivity } else { menuUnblock.setVisible(false); } - if (!this.getSelectedConversation().getAccount().getXmppConnection().getFeatures().blocking()) { + final Account account = this.getSelectedConversation().getAccount(); + if (account.getStatus() != Account.State.ONLINE || !account.getXmppConnection().getFeatures().blocking()) { menuBlock.setVisible(false); menuUnblock.setVisible(false); } diff --git a/src/main/java/eu/siacs/conversations/ui/EditAccountActivity.java b/src/main/java/eu/siacs/conversations/ui/EditAccountActivity.java index 47fe5964e..0dd7a2ebd 100644 --- a/src/main/java/eu/siacs/conversations/ui/EditAccountActivity.java +++ b/src/main/java/eu/siacs/conversations/ui/EditAccountActivity.java @@ -327,7 +327,7 @@ public class EditAccountActivity extends XmppActivity implements OnAccountUpdate if (mAccount == null) { showQrCode.setVisible(false); showBlocklist.setVisible(false); - } else if (!mAccount.getXmppConnection().getFeatures().blocking()) { + } else if (mAccount.getStatus() != Account.State.ONLINE || !mAccount.getXmppConnection().getFeatures().blocking()) { showBlocklist.setVisible(false); } return true; -- cgit v1.2.3 From e152ed1e0735b9502f724d6a6a1e477022c3a40d Mon Sep 17 00:00:00 2001 From: Sam Whited Date: Sat, 20 Dec 2014 20:13:13 -0500 Subject: Simplify roster handling code Merge result handling code into IqParser Fixes #20 --- .../eu/siacs/conversations/parser/IqParser.java | 153 ++++++++++----------- .../services/XmppConnectionService.java | 64 ++++----- .../java/eu/siacs/conversations/utils/Xmlns.java | 1 + 3 files changed, 97 insertions(+), 121 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/parser/IqParser.java b/src/main/java/eu/siacs/conversations/parser/IqParser.java index b77d460d7..eea8e84c5 100644 --- a/src/main/java/eu/siacs/conversations/parser/IqParser.java +++ b/src/main/java/eu/siacs/conversations/parser/IqParser.java @@ -71,103 +71,88 @@ public class IqParser extends AbstractParser implements OnIqPacketReceived { return super.avatarData(items); } + public static boolean fromServer(final Account account, final IqPacket packet) { + return packet.getFrom() == null || packet.getFrom().equals(account.getServer()) || packet.getFrom().equals(account.getJid().toBareJid()); + } + @Override public void onIqPacketReceived(final Account account, final IqPacket packet) { - if (packet.hasChild("query", "jabber:iq:roster")) { - final Jid from = packet.getFrom(); - if ((from == null) || (from.equals(account.getJid().toBareJid()))) { - final Element query = packet.findChild("query"); - this.rosterItems(account, query); + if (packet.hasChild("query", Xmlns.ROSTER) && fromServer(account, packet)) { + final Element query = packet.findChild("query"); + // If this is in response to a query for the whole roster: + if (packet.getType() == IqPacket.TYPE_RESULT) { + account.getRoster().markAllAsNotInRoster(); } - } else if (packet.hasChild("block", Xmlns.BLOCKING) || packet.hasChild("blocklist", Xmlns.BLOCKING)) { - // Only accept block list changes from the server. - // The server should probably prevent other people from faking a blocklist push, - // but just in case let's prevent it client side as well. - final Jid from = packet.getFrom(); - if (from == null || from.equals(account.getServer()) || from.equals(account.getJid().toBareJid())) { - Log.d(Config.LOGTAG, "Received blocklist update from server"); - final Element blocklist = packet.findChild("blocklist", Xmlns.BLOCKING); - final Element block = packet.findChild("block", Xmlns.BLOCKING); - final Collection items = blocklist != null ? blocklist.getChildren() : - (block != null ? block.getChildren() : null); - // If this is a response to a blocklist query, clear the block list and replace with the new one. - // Otherwise, just update the existing blocklist. - if (packet.getType() == IqPacket.TYPE_RESULT) { - account.clearBlocklist(); - } - if (items != null) { - final Collection jids = new ArrayList<>(items.size()); - // Create a collection of Jids from the packet - for (final Element item : items) { - if (item.getName().equals("item")) { - final Jid jid = item.getAttributeAsJid("jid"); - if (jid != null) { - jids.add(jid); - } + this.rosterItems(account, query); + } else if ((packet.hasChild("block", Xmlns.BLOCKING) || packet.hasChild("blocklist", Xmlns.BLOCKING)) && + fromServer(account, packet)) { + // Block list or block push. + Log.d(Config.LOGTAG, "Received blocklist update from server"); + final Element blocklist = packet.findChild("blocklist", Xmlns.BLOCKING); + final Element block = packet.findChild("block", Xmlns.BLOCKING); + final Collection items = blocklist != null ? blocklist.getChildren() : + (block != null ? block.getChildren() : null); + // If this is a response to a blocklist query, clear the block list and replace with the new one. + // Otherwise, just update the existing blocklist. + if (packet.getType() == IqPacket.TYPE_RESULT) { + account.clearBlocklist(); + } + if (items != null) { + final Collection jids = new ArrayList<>(items.size()); + // Create a collection of Jids from the packet + for (final Element item : items) { + if (item.getName().equals("item")) { + final Jid jid = item.getAttributeAsJid("jid"); + if (jid != null) { + jids.add(jid); } } - account.getBlocklist().addAll(jids); } - // Update the UI - mXmppConnectionService.updateBlocklistUi(OnUpdateBlocklist.Status.BLOCKED); - } else { - Log.d(Config.LOGTAG, "Received blocklist update from invalid jid: " + from.toString()); + account.getBlocklist().addAll(jids); } - } else if (packet.hasChild("unblock", Xmlns.BLOCKING)) { - final Jid from = packet.getFrom(); - if ((from == null || from.equals(account.getServer()) || from.equals(account.getJid().toBareJid())) && - packet.getType() == IqPacket.TYPE_SET) { - Log.d(Config.LOGTAG, "Received unblock update from server"); - final Collection items = packet.getChildren().get(0).getChildren(); - if (items.size() == 0) { - // No children to unblock == unblock all - account.getBlocklist().clear(); - } else { - final Collection jids = new ArrayList<>(items.size()); - for (final Element item : items) { - if (item.getName().equals("item")) { - final Jid jid = item.getAttributeAsJid("jid"); - if (jid != null) { - jids.add(jid); - } + // Update the UI + mXmppConnectionService.updateBlocklistUi(OnUpdateBlocklist.Status.BLOCKED); + } else if (packet.hasChild("unblock", Xmlns.BLOCKING) && + fromServer(account, packet) && packet.getType() == IqPacket.TYPE_SET) { + Log.d(Config.LOGTAG, "Received unblock update from server"); + final Collection items = packet.findChild("unblock", Xmlns.BLOCKING).getChildren(); + if (items.size() == 0) { + // No children to unblock == unblock all + account.getBlocklist().clear(); + } else { + final Collection jids = new ArrayList<>(items.size()); + for (final Element item : items) { + if (item.getName().equals("item")) { + final Jid jid = item.getAttributeAsJid("jid"); + if (jid != null) { + jids.add(jid); } } - account.getBlocklist().removeAll(jids); - mXmppConnectionService.updateBlocklistUi(OnUpdateBlocklist.Status.UNBLOCKED); - } - } else { - if (packet.getType() == IqPacket.TYPE_SET) { - Log.d(Config.LOGTAG, "Received unblock update from invalid jid " + from.toString()); - } else { - Log.d(Config.LOGTAG, "Received unblock update with invalid type " + packet.getType()); } + account.getBlocklist().removeAll(jids); } + mXmppConnectionService.updateBlocklistUi(OnUpdateBlocklist.Status.UNBLOCKED); + } else if (packet.hasChild("open", "http://jabber.org/protocol/ibb") + || packet.hasChild("data", "http://jabber.org/protocol/ibb")) { + mXmppConnectionService.getJingleConnectionManager() + .deliverIbbPacket(account, packet); + } else if (packet.hasChild("query", "http://jabber.org/protocol/disco#info")) { + final IqPacket response = mXmppConnectionService.getIqGenerator() + .discoResponse(packet); + account.getXmppConnection().sendIqPacket(response, null); + } else if (packet.hasChild("ping", "urn:xmpp:ping")) { + final IqPacket response = packet.generateRespone(IqPacket.TYPE_RESULT); + mXmppConnectionService.sendIqPacket(account, response, null); } else { - if (packet.getFrom() == null) { - Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": received iq with invalid from "+packet.toString()); - return; - } else if (packet.hasChild("open", "http://jabber.org/protocol/ibb") - || packet.hasChild("data", "http://jabber.org/protocol/ibb")) { - mXmppConnectionService.getJingleConnectionManager() - .deliverIbbPacket(account, packet); - } else if (packet.hasChild("query", "http://jabber.org/protocol/disco#info")) { - final IqPacket response = mXmppConnectionService.getIqGenerator() - .discoResponse(packet); + if ((packet.getType() == IqPacket.TYPE_GET) + || (packet.getType() == IqPacket.TYPE_SET)) { + final IqPacket response = packet.generateRespone(IqPacket.TYPE_ERROR); + final Element error = response.addChild("error"); + error.setAttribute("type", "cancel"); + error.addChild("feature-not-implemented", + "urn:ietf:params:xml:ns:xmpp-stanzas"); account.getXmppConnection().sendIqPacket(response, null); - } else if (packet.hasChild("ping", "urn:xmpp:ping")) { - final IqPacket response = packet.generateRespone(IqPacket.TYPE_RESULT); - mXmppConnectionService.sendIqPacket(account, response, null); - } else { - if ((packet.getType() == IqPacket.TYPE_GET) - || (packet.getType() == IqPacket.TYPE_SET)) { - final IqPacket response = packet.generateRespone(IqPacket.TYPE_ERROR); - final Element error = response.addChild("error"); - error.setAttribute("type", "cancel"); - error.addChild("feature-not-implemented", - "urn:ietf:params:xml:ns:xmpp-stanzas"); - account.getXmppConnection().sendIqPacket(response, null); - } - } + } } } diff --git a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java index 65c631a1d..6b0b960ae 100644 --- a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java +++ b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java @@ -74,6 +74,7 @@ import eu.siacs.conversations.utils.ExceptionHelper; import eu.siacs.conversations.utils.OnPhoneContactsLoadedListener; import eu.siacs.conversations.utils.PRNGFixes; import eu.siacs.conversations.utils.PhoneHelper; +import eu.siacs.conversations.utils.Xmlns; import eu.siacs.conversations.xml.Element; import eu.siacs.conversations.xmpp.OnBindListener; import eu.siacs.conversations.xmpp.OnContactStatusChanged; @@ -483,11 +484,11 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa this.mMemorizingTrustManager = new MemorizingTrustManager( getApplicationContext()); - int maxMemory = (int) (Runtime.getRuntime().maxMemory() / 1024); - int cacheSize = maxMemory / 8; + final int maxMemory = (int) (Runtime.getRuntime().maxMemory() / 1024); + final int cacheSize = maxMemory / 8; this.mBitmapCache = new LruCache(cacheSize) { @Override - protected int sizeOf(String key, Bitmap bitmap) { + protected int sizeOf(final String key, final Bitmap bitmap) { return bitmap.getByteCount() / 1024; } }; @@ -495,7 +496,7 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa this.databaseBackend = DatabaseBackend.getInstance(getApplicationContext()); this.accounts = databaseBackend.getAccounts(); - for (Account account : this.accounts) { + for (final Account account : this.accounts) { account.initOtrEngine(this); this.databaseBackend.readRoster(account.getRoster()); } @@ -521,7 +522,7 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa } @Override - public void onTaskRemoved(Intent rootIntent) { + public void onTaskRemoved(final Intent rootIntent) { super.onTaskRemoved(rootIntent); if (!getPreferences().getBoolean("keep_foreground_service",false)) { this.logoutAndSave(); @@ -529,7 +530,7 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa } private void logoutAndSave() { - for (Account account : accounts) { + for (final Account account : accounts) { databaseBackend.writeRoster(account.getRoster()); if (account.getXmppConnection() != null) { disconnect(account, false); @@ -791,21 +792,9 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa } else { Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": fetching roster"); } - iqPacket.query("jabber:iq:roster").setAttribute("ver", + iqPacket.query(Xmlns.ROSTER).setAttribute("ver", account.getRosterVersion()); - account.getXmppConnection().sendIqPacket(iqPacket, - new OnIqPacketReceived() { - - @Override - public void onIqPacketReceived(final Account account, - final IqPacket packet) { - final Element query = packet.findChild("query"); - if (query != null) { - account.getRoster().markAllAsNotInRoster(); - mIqParser.rosterItems(account, query); - } - } - }); + account.getXmppConnection().sendIqPacket(iqPacket, mIqParser); } public void fetchBookmarks(final Account account) { @@ -1433,7 +1422,7 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa } public void createAdhocConference(final Account account, final Iterable jids, final UiCallback callback) { - Log.d(Config.LOGTAG,account.getJid().toBareJid().toString()+": creating adhoc conference with "+ jids.toString()); + Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": creating adhoc conference with " + jids.toString()); if (account.getStatus() == Account.State.ONLINE) { try { String server = findConferenceServer(account); @@ -1637,17 +1626,17 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa return false; } - public void pushContactToServer(Contact contact) { + public void pushContactToServer(final Contact contact) { contact.resetOption(Contact.Options.DIRTY_DELETE); contact.setOption(Contact.Options.DIRTY_PUSH); - Account account = contact.getAccount(); + final Account account = contact.getAccount(); if (account.getStatus() == Account.State.ONLINE) { - boolean ask = contact.getOption(Contact.Options.ASKING); - boolean sendUpdates = contact + final boolean ask = contact.getOption(Contact.Options.ASKING); + final boolean sendUpdates = contact .getOption(Contact.Options.PENDING_SUBSCRIPTION_REQUEST) && contact.getOption(Contact.Options.PREEMPTIVE_GRANT); - IqPacket iq = new IqPacket(IqPacket.TYPE_SET); - iq.query("jabber:iq:roster").addChild(contact.asElement()); + final IqPacket iq = new IqPacket(IqPacket.TYPE_SET); + iq.query(Xmlns.ROSTER).addChild(contact.asElement()); account.getXmppConnection().sendIqPacket(iq, null); if (sendUpdates) { sendPresencePacket(account, @@ -1660,7 +1649,8 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa } } - public void publishAvatar(Account account, Uri image, + public void publishAvatar(final Account account, + final Uri image, final UiCallback callback) { final Bitmap.CompressFormat format = Config.AVATAR_FORMAT; final int size = Config.AVATAR_SIZE; @@ -1680,13 +1670,13 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa callback.error(R.string.error_saving_avatar, avatar); return; } - IqPacket packet = this.mIqGenerator.publishAvatar(avatar); + final IqPacket packet = this.mIqGenerator.publishAvatar(avatar); this.sendIqPacket(account, packet, new OnIqPacketReceived() { @Override public void onIqPacketReceived(Account account, IqPacket result) { if (result.getType() == IqPacket.TYPE_RESULT) { - IqPacket packet = XmppConnectionService.this.mIqGenerator + final IqPacket packet = XmppConnectionService.this.mIqGenerator .publishAvatarMetadata(avatar); sendIqPacket(account, packet, new OnIqPacketReceived() { @@ -1819,7 +1809,7 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa Account account = contact.getAccount(); if (account.getStatus() == Account.State.ONLINE) { IqPacket iq = new IqPacket(IqPacket.TYPE_SET); - Element item = iq.query("jabber:iq:roster").addChild("item"); + Element item = iq.query(Xmlns.ROSTER).addChild("item"); item.setAttribute("jid", contact.getJid().toString()); item.setAttribute("subscription", "remove"); account.getXmppConnection().sendIqPacket(iq, null); @@ -2027,12 +2017,12 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa } public List getKnownHosts() { - List hosts = new ArrayList<>(); - for (Account account : getAccounts()) { + final List hosts = new ArrayList<>(); + for (final Account account : getAccounts()) { if (!hosts.contains(account.getServer().toString())) { hosts.add(account.getServer().toString()); } - for (Contact contact : account.getRoster().getContacts()) { + for (final Contact contact : account.getRoster().getContacts()) { if (contact.showInRoster()) { final String server = contact.getServer().toString(); if (server != null && !hosts.contains(server)) { @@ -2045,10 +2035,10 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa } public List getKnownConferenceHosts() { - ArrayList mucServers = new ArrayList<>(); - for (Account account : accounts) { + final ArrayList mucServers = new ArrayList<>(); + for (final Account account : accounts) { if (account.getXmppConnection() != null) { - String server = account.getXmppConnection().getMucServer(); + final String server = account.getXmppConnection().getMucServer(); if (server != null && !mucServers.contains(server)) { mucServers.add(server); } diff --git a/src/main/java/eu/siacs/conversations/utils/Xmlns.java b/src/main/java/eu/siacs/conversations/utils/Xmlns.java index 932e48ae6..acea2e562 100644 --- a/src/main/java/eu/siacs/conversations/utils/Xmlns.java +++ b/src/main/java/eu/siacs/conversations/utils/Xmlns.java @@ -2,4 +2,5 @@ package eu.siacs.conversations.utils; public final class Xmlns { public static final String BLOCKING = "urn:xmpp:blocking"; + public static final String ROSTER = "jabber:iq:roster"; } -- cgit v1.2.3 From d5f99560ff13a3da42b0173fd73add653e08824b Mon Sep 17 00:00:00 2001 From: iNPUTmice Date: Tue, 23 Dec 2014 18:09:29 +0100 Subject: made method private that is no longer required to be public --- src/main/java/eu/siacs/conversations/parser/IqParser.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/eu/siacs/conversations/parser/IqParser.java b/src/main/java/eu/siacs/conversations/parser/IqParser.java index eea8e84c5..5ed3b5b4f 100644 --- a/src/main/java/eu/siacs/conversations/parser/IqParser.java +++ b/src/main/java/eu/siacs/conversations/parser/IqParser.java @@ -22,7 +22,7 @@ public class IqParser extends AbstractParser implements OnIqPacketReceived { super(service); } - public void rosterItems(final Account account, final Element query) { + private void rosterItems(final Account account, final Element query) { final String version = query.getAttribute("ver"); if (version != null) { account.getRoster().setVersion(version); -- cgit v1.2.3