From 5cd8e349ec4354853bc4644ad3e16984227aca65 Mon Sep 17 00:00:00 2001 From: AL-Session <160798022+AL-Session@users.noreply.github.com> Date: Tue, 21 Jan 2025 09:57:59 +1100 Subject: [PATCH] Fix/attachment filename finetuning (#886) * Initial commit * Initial attempt at modernising PhassphrasePromptActivity and BiometricSecretProvider * Commit before refactoring to cache shared content Uri * External sharing with fingerprint-locked device now working * Adjust PassphrasePromptActivity to not allow multiple instances if you have one then external share via session * Cleanup and documentation * End of day push * Yeah, now external sharing ONLY works on a Pixel 7a and not most other things - will need to rework this in 2025 =/ * Added forced READ_URI permission to intent - works for external share of links now, but not images at present * WIP * Working, now to clean up * Broke then fixed * End of day push * Now working on Pixel 7a also * Commit before refactor * Refactor WIP * Refactored PassphraseRequiredActionBarActivity to ScreenLockActionBarActivity, amongst other things * Cleanup * Tiny fix * Further cleanup * WIP filename fix * Fixed GIF saving amongst other things * Cleanup for PR * Fixed overlooked drawable ID change * Cleanup * PR feedback * PR feedback * PR feedback * PR feedback * PR feedback * Fix startup crash & successful unlock drawable * Adjusted fileprovider back to previous name to keep inline with avatar and share logs file providers * Cleaned up ShareActivity null-checks via lateinits * Fixed up ScreenLockActivity so we don't get stuck & inform user of system-level biometric lockouts * Fixed ShareActivity.onCreate to match OG behaviour * Added filename handling for video files * All working - commit before cleanup * Cleaned up * Addressed PR feedback regarding biometric unlock * Re-fixed external share filenames following Media class adjustment today * Cleanup * Fixed Giphy GIF filenames * PR cleanup * PR feedback * Added comment * Reverted onBackPressedDispatcher change * Added voice message filename generation method to FilenameUtils * Push before refactor * WIP * Streamline filename creation * Cleanup * Enforce non-null status on extracted filename * Adjusted Attachments to force filenames & removed reliance on Uri filename extraction * End of day push * Added synthesized filename creation for pre-existing voice messages that lack a filename * Cleanup and refactor mechanism for legacy voice message filename synthesis * Remove unnecessary call to getFilenameFromUri if we can extract the filename from the attachment * Added filename synthesis on input from legacy Session Android clients that provide null filenames * Added some additional mime-types to our incoming 'fromProto' filename generation - should be very comprehensive now * Leaving logic on the app side Removed logic from libsession Sanitising file names before saving them to storage Made sure we don't displa a date when getting filename from Slide otherwise it'll keep changing * Cleaned up * Using the URI's timestamp when possible * WIP * Working - commiting before cleanup * Cleaned up for PR --------- Co-authored-by: alansley Co-authored-by: ThomasSession --- .../securesms/database/Storage.kt | 43 +++--- .../thoughtcrime/securesms/mms/AudioSlide.kt | 5 +- .../org/thoughtcrime/securesms/mms/Slide.kt | 8 +- .../securesms/util/FilenameUtils.kt | 122 ++++++++++-------- 4 files changed, 98 insertions(+), 80 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt b/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt index 7798535937..4d93db3390 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt @@ -4,13 +4,18 @@ import android.content.Context import android.net.Uri import com.goterl.lazysodium.utils.KeyPair import dagger.hilt.android.qualifiers.ApplicationContext +import java.security.MessageDigest +import javax.inject.Inject +import javax.inject.Singleton import network.loki.messenger.libsession_util.ConfigBase.Companion.PRIORITY_HIDDEN import network.loki.messenger.libsession_util.ConfigBase.Companion.PRIORITY_PINNED import network.loki.messenger.libsession_util.ConfigBase.Companion.PRIORITY_VISIBLE import network.loki.messenger.libsession_util.util.BaseCommunityInfo +import network.loki.messenger.libsession_util.util.Contact as LibSessionContact import network.loki.messenger.libsession_util.util.ExpiryMode import network.loki.messenger.libsession_util.util.GroupDisplayInfo import network.loki.messenger.libsession_util.util.GroupInfo +import network.loki.messenger.libsession_util.util.GroupMember as LibSessionGroupMember import network.loki.messenger.libsession_util.util.UserPic import org.session.libsession.avatars.AvatarHelper import org.session.libsession.database.MessageDataProvider @@ -89,11 +94,6 @@ import org.thoughtcrime.securesms.groups.OpenGroupManager import org.thoughtcrime.securesms.mms.PartAuthority import org.thoughtcrime.securesms.util.FilenameUtils import org.thoughtcrime.securesms.util.SessionMetaProtocol -import java.security.MessageDigest -import javax.inject.Inject -import javax.inject.Singleton -import network.loki.messenger.libsession_util.util.Contact as LibSessionContact -import network.loki.messenger.libsession_util.util.GroupMember as LibSessionGroupMember private const val TAG = "Storage" @@ -220,17 +220,11 @@ open class Storage @Inject constructor( } } - override fun getUserPublicKey(): String? { - return preferences.getLocalNumber() - } + override fun getUserPublicKey(): String? { return preferences.getLocalNumber() } - override fun getUserX25519KeyPair(): ECKeyPair { - return lokiAPIDatabase.getUserX25519KeyPair() - } + override fun getUserX25519KeyPair(): ECKeyPair { return lokiAPIDatabase.getUserX25519KeyPair() } - override fun getUserED25519KeyPair(): KeyPair? { - return KeyPairUtilities.getUserED25519KeyPair(context) - } + override fun getUserED25519KeyPair(): KeyPair? { return KeyPairUtilities.getUserED25519KeyPair(context) } override fun getUserProfile(): Profile { val displayName = TextSecurePreferences.getProfileName(context) @@ -296,11 +290,7 @@ open class Storage @Inject constructor( val info = lokiMessageDatabase.getSendersForHashes(threadId, hashes) - if (senderIsMe) { - return info.all { it.isOutgoing } - } else { - return info.all { it.sender == sender } - } + return if (senderIsMe) info.all { it.isOutgoing } else info.all { it.sender == sender } } override fun deleteMessagesByHash(threadId: Long, hashes: List) { @@ -422,11 +412,20 @@ open class Storage @Inject constructor( val expiryMode = message.expiryMode val expiresInMillis = expiryMode.expiryMillis val expireStartedAt = if (expiryMode is ExpiryMode.AfterSend) message.sentTimestamp!! else 0 + + if (message.isMediaMessage() || attachments.isNotEmpty()) { - // sanitise attachments with missing names - for(attachment in attachments.filter { it.filename.isNullOrEmpty() }) { - attachment.filename = FilenameUtils.getFilenameFromUri(context, Uri.parse(attachment.url), attachment.contentType) + + // Sanitise attachments with missing names + for (attachment in attachments.filter { it.filename.isNullOrEmpty() }) { + + // Unfortunately we have multiple Attachment classes, but only `SignalAttachment` has the `isVoiceNote` property which we can + // use to differentiate between an audio-file with no filename and a voice-message with no file-name, so we convert to that + // and pass it through. + val signalAttachment = attachment.toSignalAttachment() + attachment.filename = FilenameUtils.getFilenameFromUri(context, Uri.parse(attachment.url), attachment.contentType, signalAttachment) } + val quote: Optional = if (quotes != null) Optional.of(quotes) else Optional.absent() val linkPreviews: Optional> = if (linkPreview.isEmpty()) Optional.absent() else Optional.of(linkPreview.mapNotNull { it!! }) val insertResult = if (isUserSender || isUserBlindedSender) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/mms/AudioSlide.kt b/app/src/main/java/org/thoughtcrime/securesms/mms/AudioSlide.kt index 6935d8b5d0..1d39bd6f9f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mms/AudioSlide.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/mms/AudioSlide.kt @@ -19,6 +19,7 @@ package org.thoughtcrime.securesms.mms import android.content.Context import android.content.res.Resources import android.net.Uri +import android.util.Log import androidx.annotation.DrawableRes import network.loki.messenger.R import org.session.libsession.messaging.sending_receiving.attachments.Attachment @@ -46,7 +47,9 @@ class AudioSlide : Slide { override fun hasAudio() = true // Legacy voice messages don't have filenames at all - so should we come across one we must synthesize a filename using the delivery date obtained from the attachment - override fun generateSuitableFilenameFromUri(context: Context, uri: Uri?) = FilenameUtils.constructVoiceMessageFilenameFromAttachment(context, attachment) + override fun generateSuitableFilenameFromUri(context: Context, uri: Uri?): String { + return FilenameUtils.constructAudioMessageFilenameFromAttachment(context, attachment) + } @DrawableRes override fun getPlaceholderRes(theme: Resources.Theme?) = R.drawable.ic_volume_2 diff --git a/app/src/main/java/org/thoughtcrime/securesms/mms/Slide.kt b/app/src/main/java/org/thoughtcrime/securesms/mms/Slide.kt index ce5fe3d8bb..961b4e11db 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mms/Slide.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/mms/Slide.kt @@ -74,12 +74,14 @@ abstract class Slide(@JvmField protected val context: Context, protected val att get() = Optional.fromNullable(attachment.caption) val filename: String by lazy { - if(attachment.filename.isNullOrEmpty()) generateSuitableFilenameFromUri(context, attachment.dataUri) else attachment.filename + if (attachment.filename.isNullOrEmpty()) generateSuitableFilenameFromUri(context, attachment.dataUri) else attachment.filename } // Note: All slide types EXCEPT AudioSlide use this technique to synthesize a filename from a Uri - however AudioSlide has - // its own custom version to handle legacy voice messages which lack filenames altogether. - open fun generateSuitableFilenameFromUri(context: Context, uri: Uri?) = FilenameUtils.getFilenameFromUri(context, attachment.dataUri, attachment.contentType) + // its own custom version to handle legacy voice messages which lack filenames. + open fun generateSuitableFilenameFromUri(context: Context, uri: Uri?): String { + return FilenameUtils.getFilenameFromUri(context, attachment.dataUri, attachment.contentType) + } val fastPreflightId: String? get() = attachment.fastPreflightId diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/FilenameUtils.kt b/app/src/main/java/org/thoughtcrime/securesms/util/FilenameUtils.kt index 8648cde6f4..4a50dd5f75 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/FilenameUtils.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/util/FilenameUtils.kt @@ -9,7 +9,6 @@ import network.loki.messenger.R import org.session.libsession.messaging.sending_receiving.attachments.Attachment import org.session.libsignal.utilities.Log - object FilenameUtils { private const val TAG = "FilenameUtils" @@ -18,43 +17,31 @@ object FilenameUtils { return dateFormatter.format( timestamp ?: System.currentTimeMillis() ) } + // Filename for when we take a photo from within Session @JvmStatic fun constructPhotoFilename(context: Context): String = "${context.getString(R.string.app_name)}-Photo-${getFormattedDate()}.jpg" + // Filename for when we create a new voice message @JvmStatic fun constructNewVoiceMessageFilename(context: Context): String = context.getString(R.string.app_name) + "-" + context.getString(R.string.messageVoice).replace(" ", "") + "_${getFormattedDate()}" + ".aac" - // Method to synthesize a suitable filename for a legacy voice message that has no filename whatsoever + // Method to synthesize a suitable filename for a voice message that we have been sent. + // Note: If we have a file as an attachment then it has a `isVoiceNote` property which @JvmStatic - fun constructVoiceMessageFilenameFromAttachment(context: Context, attachment: Attachment): String { - var constructedFilename = "" - val appNameString = context.getString(R.string.app_name) - val voiceMessageString = context.getString(R.string.messageVoice).replace(" ", "") + fun constructAudioMessageFilenameFromAttachment(context: Context, attachment: Attachment): String { + // Try and get the file extension, e.g., from "audio/aac" extract the "aac" part etc. + val fileExtensionSegments = attachment.contentType.split("/") + val fileExtension = if (fileExtensionSegments.size == 2) fileExtensionSegments[1] else "" // We SHOULD always have a uri path - but it's not guaranteed val uriPath = attachment.dataUri?.path - if (uriPath != null) { - // The Uri path contains a timestamp for when the attachment was written, typically in the form "/part/1736914338425/4", - // where the middle element ("1736914338425" in this case) equates to: Wednesday, 15 January 2025 15:12:18.425 (in the GST+11 timezone). - // The final "/4" is likely the part number. - attachment.dataUri!!.pathSegments.let { segments -> - // If we can extract a timestamp from the Uri path then we'll use that in our voice message filename synthesis - if (segments.size >= 2 && segments[1].toLongOrNull() != null) { - val extractedTimestamp = segments[1].toLong() - constructedFilename = appNameString + "-" + voiceMessageString + "_${getFormattedDate(extractedTimestamp)}" + ".aac" - } - } - } - return if (constructedFilename.isEmpty()) { - // If we didn't have a Uri path or couldn't extract the timestamp then we'll call the voice message "Session-VoiceMessage.aac".. - // Note: On save, should a file with this name already exist it will have an incremental number appended, e.g., - // Session-VoiceMessage-1.aac, Session-VoiceMessage-2.aac etc. - "$appNameString-$voiceMessageString.aac" - } else { - // ..otherwise we'll return a more accurate filename such as "Session-VoiceMessage_2025-01-15-151218.aac". - constructedFilename - } + val timestamp = if (uriPath.isNullOrEmpty()) System.currentTimeMillis() else getTimestampFromUri(uriPath) + + // Return the filename using either the "VoiceMessage" or "Audio" string depending on the attachment type + val appNameString = context.getString(R.string.app_name) + val audioTypeString = if (attachment.isVoiceNote) context.getString(R.string.messageVoice).replace(" ", "") else context.getString(R.string.audio) + return "$appNameString-${audioTypeString}_${getFormattedDate(timestamp)}.$fileExtension" } // As all picked media now has a mandatory filename this method should never get called - but it's here as a last line of defence @@ -62,19 +49,29 @@ object FilenameUtils { fun constructFallbackMediaFilenameFromMimeType( context: Context, mimeType: String?, - fileTimestamp: Long? + timestamp: Long? ): String { - val timestamp = "_${getFormattedDate(fileTimestamp)}" + // If we couldn't extract a timestamp from a Uri then the best we can do is use now. + // Note: Once a file is created with this timestamp it is maintained with that timestamp so + // we do not have issues such as saving the file multiple times resulting in multiple filenames + // where each file uses the "now" timestamp it was saved at (although multiple files will + // have -1, -2, -3 etc. suffixes to prevent overwriting any file). + val guaranteedTimestamp = timestamp ?: System.currentTimeMillis() + val formattedDate = "_${getFormattedDate(guaranteedTimestamp)}" + val fileExtension = mimeType?.split("/")?.get(1) ?: "" return if (MediaUtil.isVideoType(mimeType)) { - "${context.getString(R.string.app_name)}-${context.getString(R.string.video)}$timestamp" // Session-Video_ + "${context.getString(R.string.app_name)}-${context.getString(R.string.video)}$formattedDate.$fileExtension" // Session-Video_ } else if (MediaUtil.isGif(mimeType)) { - "${context.getString(R.string.app_name)}-${context.getString(R.string.gif)}$timestamp" // Session-GIF_ + "${context.getString(R.string.app_name)}-${context.getString(R.string.gif)}$formattedDate.$fileExtension" // Session-GIF_ } else if (MediaUtil.isImageType(mimeType)) { - "${context.getString(R.string.app_name)}-${context.getString(R.string.image)}$timestamp" // Session-Image_ - } else { - Log.d(TAG, "Asked to construct a filename for an unsupported media type: $mimeType.") - "${context.getString(R.string.app_name)}$timestamp" // Session_ - best we can do + "${context.getString(R.string.app_name)}-${context.getString(R.string.image)}$formattedDate.$fileExtension" // Session-Image_ + } else if (MediaUtil.isAudioType(mimeType)) { + "${context.getString(R.string.app_name)}-${context.getString(R.string.audio)}$formattedDate.$fileExtension" // Session-Audio_ + } + else { + Log.i(TAG, "Asked to construct a filename for an unsupported media type: $mimeType.") + "${context.getString(R.string.app_name)}$formattedDate.$fileExtension" // Session_ - potentially no file extension, but it's the best we can do with limited data } } @@ -86,7 +83,9 @@ object FilenameUtils { // null from this method means that the calling code must construct a suitable placeholder filename. @JvmStatic @JvmOverloads // Force creation of two versions of this method - one with and one without the mimeType param - fun getFilenameFromUri(context: Context, uri: Uri?, mimeType: String? = null): String { + fun getFilenameFromUri(context: Context, uri: Uri?, mimeType: String? = null, attachment: Attachment? = null): String { + Exception().printStackTrace() + var extractedFilename: String? = null if (uri != null) { @@ -112,37 +111,52 @@ object FilenameUtils { // uri path: /blob/multi-session-disk/image/jpeg/cat.jpeg/3050/3a507d6a-f2f9-41d1-97a0-319de47e3a8d // // from which we'd want to extract the filename "cat.jpeg". - if (extractedFilename.isNullOrEmpty() && uri.path != null) { - extractedFilename = attemptUriPathExtraction(uri.path!!) + if (extractedFilename.isNullOrEmpty() && uri.path != null && uri.path!!.contains("/blob/")) { + // Split the path by "/" then traverse the segments in reverse order looking for the first one containing a dot + val segments = uri.path?.split("/") + + // If the uri path was not in the blob format extractedFilename will still be null and we'll continue on to our next + // filename synthesis technique. + extractedFilename = segments?.asReversed()?.firstOrNull { it.contains('.') } } } - // Uri filename extraction failed - synthesize a filename from the media's MIME type. - // Note: Giphy picked GIFs will use this to get a filename like "Session-GIF-" - but pre-saved GIFs - // chosen via the file-picker or similar will use the existing saved filename as they will be caught in - // the filename extraction code above. + // Uri filename extraction failed - synthesize a filename from the media's MIME type if (extractedFilename.isNullOrEmpty()) { - extractedFilename = constructFallbackMediaFilenameFromMimeType(context, mimeType, getTimestampFromUri(uri?.path)) + + if (attachment == null) { + val timestamp = if (uri?.path.isNullOrEmpty()) null else getTimestampFromUri(uri.path!!) + extractedFilename = constructFallbackMediaFilenameFromMimeType(context, mimeType, timestamp) + } else { + // If the mimetype is audio then we generate a filename which contain "VoiceMessage" or "Audio" + // based on the attachment's `isVoiceNote` flag.. + extractedFilename = if (mimeType?.contains("audio") == true) { + constructAudioMessageFilenameFromAttachment(context, attachment) + } else { + // ..otherwise we just do the best we can from the mime type (if any). + constructFallbackMediaFilenameFromMimeType(context, mimeType, null) + } + } } return extractedFilename!! } - private fun attemptUriPathExtraction(uriPath: String): String? { - // Split the path by "/" then traverse the segments in reverse order looking for the first one containing a dot + // Uri paths comes in a variety of formats - if we have the right format, such as "/part/1736914338425/4", then we can + // extract the incoming file timestamp from it. + private fun getTimestampFromUri(uriPath: String): Long? { val segments = uriPath.split("/") - val extractedFilename = segments.asReversed().firstOrNull { it.contains('.') } - // If found, return the identified filename, otherwise we'll be returning null - return extractedFilename - } + // We cannot extract a timestamp from a uri path like "/file/6921609917390343" because that large number is not a timestamp + val uriPathStartsWithFile = uriPath.startsWith("/file/") == true + if (uriPathStartsWithFile) return null - private fun getTimestampFromUri(uriPath: String?): Long?{ - val segments = uriPath?.split("/") - // we assume that at this stage we have a format that looks like /part/1111111111/123 with 1111111111 being the creation date timestamp + // But if we have a uri path in a format like "/part/1736914338425/4" then we CAN extract that timestamp (the middle value) + val uriPathStartsWithPart = uriPath.startsWith("/part/") == true + if (!uriPathStartsWithPart) return null return try { - segments?.getOrNull(2)?.toLong() - } catch (e: Exception){ + segments.getOrNull(2)?.toLong() + } catch (e: Exception) { null } }