fixed caps hash generation for empty form values

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/entities/Presence.java               |   4 
src/main/java/eu/siacs/conversations/entities/ServiceDiscoveryResult.java | 278 
src/main/java/eu/siacs/conversations/services/XmppConnectionService.java  |  27 
src/main/java/eu/siacs/conversations/xmpp/forms/Field.java                |   5 
4 files changed, 156 insertions(+), 158 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/entities/Presence.java 🔗

@@ -1,5 +1,7 @@
 package eu.siacs.conversations.entities;
 
+import android.support.annotation.NonNull;
+
 import java.lang.Comparable;
 import java.util.Locale;
 
@@ -62,7 +64,7 @@ public class Presence implements Comparable {
 		return new Presence(Status.fromShowString(show), ver, hash, node, message);
 	}
 
-	public int compareTo(Object other) {
+	public int compareTo(@NonNull Object other) {
 		return this.status.compareTo(((Presence)other).status);
 	}
 

src/main/java/eu/siacs/conversations/entities/ServiceDiscoveryResult.java 🔗

@@ -2,7 +2,10 @@ package eu.siacs.conversations.entities;
 
 import android.content.ContentValues;
 import android.database.Cursor;
+import android.support.annotation.NonNull;
 import android.util.Base64;
+import android.util.Log;
+
 import java.io.UnsupportedEncodingException;
 import java.lang.Comparable;
 import java.security.MessageDigest;
@@ -16,6 +19,7 @@ import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 
+import eu.siacs.conversations.Config;
 import eu.siacs.conversations.xml.Element;
 import eu.siacs.conversations.xml.Namespace;
 import eu.siacs.conversations.xmpp.forms.Data;
@@ -27,95 +31,11 @@ public class ServiceDiscoveryResult {
 	public static final String HASH = "hash";
 	public static final String VER = "ver";
 	public static final String RESULT = "result";
-
-	protected static String blankNull(String s) {
-		return s == null ? "" : s;
-	}
-
-	public static class Identity implements Comparable {
-		protected final String category;
-		protected final String type;
-		protected final String lang;
-		protected final String name;
-
-		public Identity(final String category, final String type, final String lang, final String name) {
-			this.category = category;
-			this.type = type;
-			this.lang = lang;
-			this.name = name;
-		}
-
-		public Identity(final Element el) {
-			this(
-				el.getAttribute("category"),
-				el.getAttribute("type"),
-				el.getAttribute("xml:lang"),
-				el.getAttribute("name")
-			);
-		}
-
-		public Identity(final JSONObject o) {
-
-			this(
-				o.optString("category", null),
-				o.optString("type", null),
-				o.optString("lang", null),
-				o.optString("name", null)
-			);
-		}
-
-		public String getCategory() {
-			return this.category;
-		}
-
-		public String getType() {
-			return this.type;
-		}
-
-		public String getLang() {
-			return this.lang;
-		}
-
-		public String getName() {
-			return this.name;
-		}
-
-		public int compareTo(Object other) {
-			Identity o = (Identity)other;
-			int r = blankNull(this.getCategory()).compareTo(blankNull(o.getCategory()));
-			if(r == 0) {
-				r = blankNull(this.getType()).compareTo(blankNull(o.getType()));
-			}
-			if(r == 0) {
-				r = blankNull(this.getLang()).compareTo(blankNull(o.getLang()));
-			}
-			if(r == 0) {
-				r = blankNull(this.getName()).compareTo(blankNull(o.getName()));
-			}
-
-			return r;
-		}
-
-		public JSONObject toJSON() {
-			try {
-				JSONObject o = new JSONObject();
-				o.put("category", this.getCategory());
-				o.put("type", this.getType());
-				o.put("lang", this.getLang());
-				o.put("name", this.getName());
-				return o;
-			} catch(JSONException e) {
-				return null;
-			}
-		}
-	}
-
 	protected final String hash;
 	protected final byte[] ver;
-	protected final List<Identity> identities;
 	protected final List<String> features;
 	protected final List<Data> forms;
-
+	private final List<Identity> identities;
 	public ServiceDiscoveryResult(final IqPacket packet) {
 		this.identities = new ArrayList<>();
 		this.features = new ArrayList<>();
@@ -140,8 +60,7 @@ public class ServiceDiscoveryResult {
 		}
 		this.ver = this.mkCapHash();
 	}
-
-	public ServiceDiscoveryResult(String hash, byte[] ver, JSONObject o) throws JSONException {
+	private ServiceDiscoveryResult(String hash, byte[] ver, JSONObject o) throws JSONException {
 		this.identities = new ArrayList<>();
 		this.features = new ArrayList<>();
 		this.forms = new ArrayList<>();
@@ -162,21 +81,37 @@ public class ServiceDiscoveryResult {
 		}
 		JSONArray forms = o.optJSONArray("forms");
 		if (forms != null) {
-			for(int i = 0; i < forms.length(); i++) {
+			for (int i = 0; i < forms.length(); i++) {
 				this.forms.add(createFormFromJSONObject(forms.getJSONObject(i)));
 			}
 		}
 	}
 
+	public ServiceDiscoveryResult(Cursor cursor) throws JSONException {
+		this(
+				cursor.getString(cursor.getColumnIndex(HASH)),
+				Base64.decode(cursor.getString(cursor.getColumnIndex(VER)), Base64.DEFAULT),
+				new JSONObject(cursor.getString(cursor.getColumnIndex(RESULT)))
+		);
+	}
+
+	private static String clean(String s) {
+		return s.replace("<","&lt;");
+	}
+
+	private static String blankNull(String s) {
+		return s == null ? "" : clean(s);
+	}
+
 	private static Data createFormFromJSONObject(JSONObject o) {
 		Data data = new Data();
 		JSONArray names = o.names();
-		for(int i = 0; i < names.length(); ++i) {
+		for (int i = 0; i < names.length(); ++i) {
 			try {
 				String name = names.getString(i);
 				JSONArray jsonValues = o.getJSONArray(name);
 				ArrayList<String> values = new ArrayList<>(jsonValues.length());
-				for(int j = 0; j < jsonValues.length(); ++j) {
+				for (int j = 0; j < jsonValues.length(); ++j) {
 					values.add(jsonValues.getString(j));
 				}
 				data.put(name, values);
@@ -189,14 +124,14 @@ public class ServiceDiscoveryResult {
 
 	private static JSONObject createJSONFromForm(Data data) {
 		JSONObject object = new JSONObject();
-		for(Field field : data.getFields()) {
+		for (Field field : data.getFields()) {
 			try {
 				JSONArray jsonValues = new JSONArray();
-				for(String value : field.getValues()) {
+				for (String value : field.getValues()) {
 					jsonValues.put(value);
 				}
 				object.put(field.getFieldName(), jsonValues);
-			} catch(Exception e) {
+			} catch (Exception e) {
 				e.printStackTrace();
 			}
 		}
@@ -204,7 +139,7 @@ public class ServiceDiscoveryResult {
 			JSONArray jsonValues = new JSONArray();
 			jsonValues.put(data.getFormType());
 			object.put(Data.FORM_TYPE, jsonValues);
-		} catch(Exception e) {
+		} catch (Exception e) {
 			e.printStackTrace();
 		}
 		return object;
@@ -214,14 +149,6 @@ public class ServiceDiscoveryResult {
 		return new String(Base64.encode(this.ver, Base64.DEFAULT)).trim();
 	}
 
-	public ServiceDiscoveryResult(Cursor cursor) throws JSONException {
-		this(
-			cursor.getString(cursor.getColumnIndex(HASH)),
-			Base64.decode(cursor.getString(cursor.getColumnIndex(VER)), Base64.DEFAULT),
-			new JSONObject(cursor.getString(cursor.getColumnIndex(RESULT)))
-		);
-	}
-
 	public List<Identity> getIdentities() {
 		return this.identities;
 	}
@@ -231,9 +158,9 @@ public class ServiceDiscoveryResult {
 	}
 
 	public boolean hasIdentity(String category, String type) {
-		for(Identity id : this.getIdentities()) {
-			if((category == null || id.getCategory().equals(category)) &&
-			   (type == null || id.getType().equals(type))) {
+		for (Identity id : this.getIdentities()) {
+			if ((category == null || id.getCategory().equals(category)) &&
+					(type == null || id.getType().equals(type))) {
 				return true;
 			}
 		}
@@ -242,9 +169,9 @@ public class ServiceDiscoveryResult {
 	}
 
 	public String getExtendedDiscoInformation(String formType, String name) {
-		for(Data form : this.forms) {
+		for (Data form : this.forms) {
 			if (formType.equals(form.getFormType())) {
-				for(Field field: form.getFields()) {
+				for (Field field : form.getFields()) {
 					if (name.equals(field.getFieldName())) {
 						return field.getValue();
 					}
@@ -254,50 +181,42 @@ public class ServiceDiscoveryResult {
 		return null;
 	}
 
-	protected byte[] mkCapHash() {
+	private byte[] mkCapHash() {
 		StringBuilder s = new StringBuilder();
 
 		List<Identity> identities = this.getIdentities();
 		Collections.sort(identities);
 
-		for(Identity id : identities) {
-			s.append(
-				blankNull(id.getCategory()) + "/" +
-				blankNull(id.getType()) + "/" +
-				blankNull(id.getLang()) + "/" +
-				blankNull(id.getName()) + "<"
-			);
+		for (Identity id : identities) {
+			s.append(blankNull(id.getCategory()))
+					.append("/")
+					.append(blankNull(id.getType()))
+					.append("/")
+					.append(blankNull(id.getLang()))
+					.append("/")
+					.append(blankNull(id.getName()))
+					.append("<");
 		}
 
 		List<String> features = this.getFeatures();
 		Collections.sort(features);
 
 		for (String feature : features) {
-			s.append(feature + "<");
+			s.append(clean(feature)).append("<");
 		}
 
-		Collections.sort(forms, new Comparator<Data>() {
-			@Override
-			public int compare(Data lhs, Data rhs) {
-				return lhs.getFormType().compareTo(rhs.getFormType());
-			}
-		});
+		Collections.sort(forms, (lhs, rhs) -> lhs.getFormType().compareTo(rhs.getFormType()));
 
-		for(Data form : forms) {
-			s.append(form.getFormType() + "<");
+		for (Data form : forms) {
+			s.append(clean(form.getFormType())).append("<");
 			List<Field> fields = form.getFields();
-			Collections.sort(fields, new Comparator<Field>() {
-				@Override
-				public int compare(Field lhs, Field rhs) {
-					return lhs.getFieldName().compareTo(rhs.getFieldName());
-				}
-			});
-			for(Field field : fields) {
-				s.append(field.getFieldName()+"<");
+			Collections.sort(fields, (lhs, rhs) -> lhs.getFieldName().compareTo(rhs.getFieldName()));
+			for (Field field : fields) {
+				s.append(clean(field.getFieldName())).append("<");
 				List<String> values = field.getValues();
 				Collections.sort(values);
-				for(String value : values) {
-					s.append(value+"<");
+				for (String value : values) {
+					s.append(blankNull(value)).append("<");
 				}
 			}
 		}
@@ -311,17 +230,17 @@ public class ServiceDiscoveryResult {
 
 		try {
 			return md.digest(s.toString().getBytes("UTF-8"));
-		} catch(UnsupportedEncodingException e) {
+		} catch (UnsupportedEncodingException e) {
 			return null;
 		}
 	}
 
-	public JSONObject toJSON() {
+	private JSONObject toJSON() {
 		try {
 			JSONObject o = new JSONObject();
 
 			JSONArray ids = new JSONArray();
-			for(Identity id : this.getIdentities()) {
+			for (Identity id : this.getIdentities()) {
 				ids.put(id.toJSON());
 			}
 			o.put("identities", ids);
@@ -329,13 +248,13 @@ public class ServiceDiscoveryResult {
 			o.put("features", new JSONArray(this.getFeatures()));
 
 			JSONArray forms = new JSONArray();
-			for(Data data : this.forms) {
+			for (Data data : this.forms) {
 				forms.put(createJSONFromForm(data));
 			}
 			o.put("forms", forms);
 
 			return o;
-		} catch(JSONException e) {
+		} catch (JSONException e) {
 			return null;
 		}
 	}
@@ -344,7 +263,86 @@ public class ServiceDiscoveryResult {
 		final ContentValues values = new ContentValues();
 		values.put(HASH, this.hash);
 		values.put(VER, getVer());
-		values.put(RESULT, this.toJSON().toString());
+		JSONObject jsonObject = toJSON();
+		values.put(RESULT, jsonObject == null ? "" : jsonObject.toString());
 		return values;
 	}
+
+	public static class Identity implements Comparable {
+		protected final String type;
+		protected final String lang;
+		protected final String name;
+		final String category;
+
+		Identity(final String category, final String type, final String lang, final String name) {
+			this.category = category;
+			this.type = type;
+			this.lang = lang;
+			this.name = name;
+		}
+
+		Identity(final Element el) {
+			this(
+					el.getAttribute("category"),
+					el.getAttribute("type"),
+					el.getAttribute("xml:lang"),
+					el.getAttribute("name")
+			);
+		}
+
+		Identity(final JSONObject o) {
+
+			this(
+					o.optString("category", null),
+					o.optString("type", null),
+					o.optString("lang", null),
+					o.optString("name", null)
+			);
+		}
+
+		public String getCategory() {
+			return this.category;
+		}
+
+		public String getType() {
+			return this.type;
+		}
+
+		public String getLang() {
+			return this.lang;
+		}
+
+		public String getName() {
+			return this.name;
+		}
+
+		public int compareTo(@NonNull Object other) {
+			Identity o = (Identity) other;
+			int r = blankNull(this.getCategory()).compareTo(blankNull(o.getCategory()));
+			if (r == 0) {
+				r = blankNull(this.getType()).compareTo(blankNull(o.getType()));
+			}
+			if (r == 0) {
+				r = blankNull(this.getLang()).compareTo(blankNull(o.getLang()));
+			}
+			if (r == 0) {
+				r = blankNull(this.getName()).compareTo(blankNull(o.getName()));
+			}
+
+			return r;
+		}
+
+		JSONObject toJSON() {
+			try {
+				JSONObject o = new JSONObject();
+				o.put("category", this.getCategory());
+				o.put("type", this.getType());
+				o.put("lang", this.getLang());
+				o.put("name", this.getName());
+				return o;
+			} catch (JSONException e) {
+				return null;
+			}
+		}
+	}
 }

src/main/java/eu/siacs/conversations/services/XmppConnectionService.java 🔗

@@ -3649,20 +3649,21 @@ public class XmppConnectionService extends Service {
 				account.inProgressDiscoFetches.add(key);
 				IqPacket request = new IqPacket(IqPacket.TYPE.GET);
 				request.setTo(jid);
-				String node = presence.getNode();
-				Element query = request.query("http://jabber.org/protocol/disco#info");
-				if (node != null) {
-					query.setAttribute("node",node);
-				}
-				Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": making disco request for " + key.second + " to " + jid+ "node="+node);
-				sendIqPacket(account, request, (a, discoPacket) -> {
-					if (discoPacket.getType() == IqPacket.TYPE.RESULT) {
-						ServiceDiscoveryResult disco1 = new ServiceDiscoveryResult(discoPacket);
-						if (presence.getVer().equals(disco1.getVer())) {
-							databaseBackend.insertDiscoveryResult(disco1);
-							injectServiceDiscorveryResult(a.getRoster(), presence.getHash(), presence.getVer(), disco1);
+				final String node = presence.getNode();
+				final String ver = presence.getVer();
+				final Element query = request.query("http://jabber.org/protocol/disco#info");
+				if (node != null && ver != null) {
+					query.setAttribute("node",node+"#"+ver);
+				}
+				Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": making disco request for " + key.second + " to " + jid);
+				sendIqPacket(account, request, (a, response) -> {
+					if (response.getType() == IqPacket.TYPE.RESULT) {
+						ServiceDiscoveryResult discoveryResult = new ServiceDiscoveryResult(response);
+						if (presence.getVer().equals(discoveryResult.getVer())) {
+							databaseBackend.insertDiscoveryResult(discoveryResult);
+							injectServiceDiscorveryResult(a.getRoster(), presence.getHash(), presence.getVer(), discoveryResult);
 						} else {
-							Log.d(Config.LOGTAG, a.getJid().asBareJid() + ": mismatch in caps for contact " + jid + " " + presence.getVer() + " vs " + disco1.getVer());
+							Log.d(Config.LOGTAG, a.getJid().asBareJid() + ": mismatch in caps for contact " + jid + " " + presence.getVer() + " vs " + discoveryResult.getVer());
 						}
 					}
 					a.inProgressDiscoFetches.remove(key);

src/main/java/eu/siacs/conversations/xmpp/forms/Field.java 🔗

@@ -58,10 +58,7 @@ public class Field extends Element {
 		List<String> values = new ArrayList<>();
 		for(Element child : getChildren()) {
 			if ("value".equals(child.getName())) {
-				String content = child.getContent();
-				if (content != null) {
-					values.add(content);
-				}
+				values.add(child.getContent());
 			}
 		}
 		return values;