refactored bodyContainsDownloadable to be more flexible

Daniel Gultsch created 10 years ago

Change summary

src/main/java/eu/siacs/conversations/crypto/PgpEngine.java        |  2 
src/main/java/eu/siacs/conversations/entities/Message.java        | 68 
src/main/java/eu/siacs/conversations/parser/MessageParser.java    |  2 
src/main/java/eu/siacs/conversations/persistance/FileBackend.java |  2 
src/main/java/eu/siacs/conversations/ui/ConversationFragment.java |  2 
5 files changed, 39 insertions(+), 37 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/crypto/PgpEngine.java

@@ -59,7 +59,7 @@ public class PgpEngine {
 								message.setEncryption(Message.ENCRYPTION_DECRYPTED);
 								final HttpConnectionManager manager = mXmppConnectionService.getHttpConnectionManager();
 								if (message.trusted()
-										&& message.bodyContainsDownloadable()
+										&& message.treatAsDownloadable() == Message.Decision.YES
 										&& manager.getAutoAcceptFileSize() > 0) {
 									manager.createNewConnection(message);
 								}

src/main/java/eu/siacs/conversations/entities/Message.java

@@ -375,8 +375,8 @@ public class Message extends AbstractEntity {
 						(message.getTimeSent() - this.getTimeSent()) <= (Config.MESSAGE_MERGE_WINDOW * 1000) &&
 						!GeoHelper.isGeoUri(message.getBody()) &&
 						!GeoHelper.isGeoUri(this.body) &&
-						!message.bodyContainsDownloadable() &&
-						!this.bodyContainsDownloadable() &&
+						message.treatAsDownloadable() == Decision.NO &&
+						this.treatAsDownloadable() == Decision.NO &&
 						!message.getBody().startsWith(ME_COMMAND) &&
 						!this.getBody().startsWith(ME_COMMAND) &&
 						!this.bodyIsHeart() &&
@@ -434,48 +434,50 @@ public class Message extends AbstractEntity {
 		return (status > STATUS_RECEIVED || (contact != null && contact.trusted()));
 	}
 
-	public boolean bodyContainsDownloadable() {
-		/**
-		 * there are a few cases where spaces result in an unwanted behavior, e.g.
-		 * "http://example.com/image.jpg text that will not be shown /abc.png"
-		 * or more than one image link in one message.
-		 */
+	public enum Decision {
+		YES,
+		NO,
+		ASK
+	}
+
+	public Decision treatAsDownloadable() {
 		if (body.trim().contains(" ")) {
-			return false;
+			return Decision.NO;
 		}
 		try {
 			URL url = new URL(body);
-			if (!url.getProtocol().equalsIgnoreCase("http")
-					&& !url.getProtocol().equalsIgnoreCase("https")) {
-				return false;
+			if (!url.getProtocol().equalsIgnoreCase("http") && !url.getProtocol().equalsIgnoreCase("https")) {
+				return Decision.NO;
 			}
-
-			String sUrlPath = url.getPath();
-			if (sUrlPath == null || sUrlPath.isEmpty()) {
-				return false;
+			String path = url.getPath();
+			if (path == null || path.isEmpty()) {
+				return Decision.NO;
 			}
 
-			int iSlashIndex = sUrlPath.lastIndexOf('/') + 1;
-
-			String sLastUrlPath = sUrlPath.substring(iSlashIndex).toLowerCase();
-
-			String[] extensionParts = sLastUrlPath.split("\\.");
-			if (extensionParts.length == 2
-					&& Arrays.asList(Downloadable.VALID_IMAGE_EXTENSIONS).contains(
-					extensionParts[extensionParts.length - 1])) {
-				return true;
-			} else if (extensionParts.length == 3
-					&& Arrays
+			String filename = path.substring(path.lastIndexOf('/') + 1).toLowerCase();
+			String[] extensionParts = filename.split("\\.");
+			String extension;
+			String ref = url.getRef();
+			if (extensionParts.length == 2) {
+				extension = extensionParts[extensionParts.length - 1];
+			} else if (extensionParts.length == 3 && Arrays
 					.asList(Downloadable.VALID_CRYPTO_EXTENSIONS)
-					.contains(extensionParts[extensionParts.length - 1])
-					&& Arrays.asList(Downloadable.VALID_IMAGE_EXTENSIONS).contains(
-					extensionParts[extensionParts.length - 2])) {
-				return true;
+					.contains(extensionParts[extensionParts.length - 1])) {
+				extension = extensionParts[extensionParts.length -2];
 			} else {
-				return false;
+				return Decision.NO;
 			}
+
+			if (Arrays.asList(Downloadable.VALID_IMAGE_EXTENSIONS).contains(extension)) {
+				return Decision.YES;
+			} else if (ref != null && ref.matches("([A-Fa-f0-9]{2}){48}")) {
+				return Decision.ASK;
+			} else {
+				return Decision.NO;
+			}
+
 		} catch (MalformedURLException e) {
-			return false;
+			return Decision.NO;
 		}
 	}
 

src/main/java/eu/siacs/conversations/parser/MessageParser.java

@@ -360,7 +360,7 @@ public class MessageParser extends AbstractParser implements
 				mXmppConnectionService.databaseBackend.createMessage(message);
 			}
 			final HttpConnectionManager manager = this.mXmppConnectionService.getHttpConnectionManager();
-			if (message.trusted() && message.bodyContainsDownloadable() && manager.getAutoAcceptFileSize() > 0) {
+			if (message.trusted() && message.treatAsDownloadable() == Message.Decision.YES && manager.getAutoAcceptFileSize() > 0) {
 				manager.createNewConnection(message);
 			} else if (!message.isRead()) {
 				mXmppConnectionService.getNotificationService().push(message);

src/main/java/eu/siacs/conversations/persistance/FileBackend.java

@@ -497,7 +497,7 @@ public class FileBackend {
 				message.setBody(url.toString()+"|"+Long.toString(file.getSize()) + '|' + imageWidth + '|' + imageHeight);
 			}
 		} else {
-			message.setBody(Long.toString(file.getSize()));
+			message.setBody(url.toString()+"|"+Long.toString(file.getSize()));
 		}
 
 	}

src/main/java/eu/siacs/conversations/ui/ConversationFragment.java

@@ -457,7 +457,7 @@ public class ConversationFragment extends Fragment implements EditMessage.Keyboa
 			}
 			if (m.getType() != Message.TYPE_TEXT
 					|| m.getDownloadable() != null
-					|| !m.bodyContainsDownloadable()) {
+					|| m.treatAsDownloadable() == Message.Decision.NO) {
 				downloadImage.setVisible(false);
 			}
 			if (!((m.getDownloadable() != null && !(m.getDownloadable() instanceof DownloadablePlaceholder))