Browse Source

Voice messages: Don't crash if mxc uri is invalid (#1756)

<!-- Please read [CONTRIBUTING.md](https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md) before submitting your pull request -->
 
## Type of change

- [ ] Feature
- [x] Bugfix
- [ ] Technical
- [ ] Other :

## Content

Code now checks whether the mx uri is invalid and handles the error condition properly.

## Motivation and context

https://github.com/vector-im/element-x-android/pull/1748#discussion_r1384557443

## Screenshots / GIFs

<!--
We have screenshot tests in the project, so attaching screenshots to a PR is not mandatory, as far as there
is a Composable Preview covering the changes. In this case, the change will appear in the file diff.
Note that all the UI composables should be covered by a Composable Preview.

Providing a video of the change is still very useful for the reviewer and for the history of the project.

You can use a table like this to show screenshots comparison.
Uncomment this markdown table below and edit the last line `|||`:
|copy screenshot of before here|copy screenshot of after here|

|Before|After|
|-|-|
|||
 -->

## Tests

<!-- Explain how you tested your development -->

- Step 1
- Step 2
- Step ...

## Tested devices

- [ ] Physical
- [ ] Emulator
- OS version(s):

## Checklist

<!-- Depending on the Pull Request content, it can be acceptable if some of the following checkboxes stay unchecked. -->

- [ ] Changes have been tested on an Android device or Android emulator with API 23
- [ ] UI change has been tested on both light and dark themes
- [ ] Accessibility has been taken into account. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
- [ ] Pull request is based on the develop branch
- [ ] Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#changelog
- [ ] Pull request includes screenshots or videos if containing UI changes
- [ ] Pull request includes a [sign off](https://matrix-org.github.io/synapse/latest/development/contributing_guide.html#sign-off)
- [ ] You've made a self review of your PR
pull/1766/head
Marco Romano 11 months ago committed by GitHub
parent
commit
26eecb33dd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 23
      features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessageMediaRepo.kt
  2. 21
      features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessageMediaRepoTest.kt

23
features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessageMediaRepo.kt

@ -83,13 +83,15 @@ class DefaultVoiceMessageMediaRepo @AssistedInject constructor( @@ -83,13 +83,15 @@ class DefaultVoiceMessageMediaRepo @AssistedInject constructor(
): DefaultVoiceMessageMediaRepo
}
override suspend fun getMediaFile(): Result<File> = if (!isInCache()) {
matrixMediaLoader.downloadMediaFile(
override suspend fun getMediaFile(): Result<File> = when {
cachedFile == null -> Result.failure(IllegalStateException("Invalid mxcUri."))
cachedFile.exists() -> Result.success(cachedFile)
else -> matrixMediaLoader.downloadMediaFile(
source = mediaSource,
mimeType = mimeType,
body = body,
).mapCatching {
val dest = cachedFilePath.apply { parentFile?.mkdirs() }
val dest = cachedFile.apply { parentFile?.mkdirs() }
// TODO By not closing the MediaFile we're leaking the rust file handle here.
// Not that big of a deal but better to avoid it someday.
if (it.toFile().renameTo(dest)) {
@ -98,13 +100,11 @@ class DefaultVoiceMessageMediaRepo @AssistedInject constructor( @@ -98,13 +100,11 @@ class DefaultVoiceMessageMediaRepo @AssistedInject constructor(
error("Failed to move file to cache.")
}
}
} else {
Result.success(cachedFilePath)
}
private val cachedFilePath: File = File("${cacheDir.path}/$CACHE_VOICE_SUBDIR/${mxcUri2FilePath(mediaSource.url)}")
private fun isInCache(): Boolean = cachedFilePath.exists()
private val cachedFile: File? = mxcUri2FilePath(mediaSource.url)?.let {
File("${cacheDir.path}/$CACHE_VOICE_SUBDIR/$it")
}
}
/**
@ -123,12 +123,9 @@ private val mxcRegex = Regex("""^mxc:\/\/([^\/]+)\/([^\/]+)$""") @@ -123,12 +123,9 @@ private val mxcRegex = Regex("""^mxc:\/\/([^\/]+)\/([^\/]+)$""")
* Sanitizes an mxcUri to be used as a relative file path.
*
* @param mxcUri the Matrix Content (mxc://) URI of the voice message.
* @return the relative file path as "<server-name>/<media-id>".
* @throws IllegalStateException if the mxcUri is invalid.
* @return the relative file path as "<server-name>/<media-id>" or null if the mxcUri is invalid.
*/
private fun mxcUri2FilePath(mxcUri: String): String = checkNotNull(mxcRegex.matchEntire(mxcUri)) {
"mxcUri2FilePath: Invalid mxcUri: $mxcUri"
}.let { match ->
private fun mxcUri2FilePath(mxcUri: String): String? = mxcRegex.matchEntire(mxcUri)?.let { match ->
buildString {
append(match.groupValues[1])
append("/")

21
features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessageMediaRepoTest.kt

@ -63,7 +63,7 @@ class DefaultVoiceMessageMediaRepoTest { @@ -63,7 +63,7 @@ class DefaultVoiceMessageMediaRepoTest {
repo.getMediaFile().let { result ->
Truth.assertThat(result.isFailure).isTrue()
result.exceptionOrNull()?.let { exception ->
result.exceptionOrNull()!!.let { exception ->
Truth.assertThat(exception).isInstanceOf(RuntimeException::class.java)
}
}
@ -116,16 +116,32 @@ class DefaultVoiceMessageMediaRepoTest { @@ -116,16 +116,32 @@ class DefaultVoiceMessageMediaRepoTest {
}
}
}
@Test
fun `invalid mxc uri returns a failure`() = runTest {
val repo = createDefaultVoiceMessageMediaRepo(
temporaryFolder = temporaryFolder,
mxcUri = INVALID_MXC_URI,
)
repo.getMediaFile().let { result ->
Truth.assertThat(result.isFailure).isTrue()
result.exceptionOrNull()!!.let { exception ->
Truth.assertThat(exception).isInstanceOf(RuntimeException::class.java)
Truth.assertThat(exception).hasMessageThat().isEqualTo("Invalid mxcUri.")
}
}
}
}
private fun createDefaultVoiceMessageMediaRepo(
temporaryFolder: TemporaryFolder,
matrixMediaLoader: MatrixMediaLoader = FakeMediaLoader(),
mxcUri: String = MXC_URI,
) = DefaultVoiceMessageMediaRepo(
cacheDir = temporaryFolder.root,
matrixMediaLoader = matrixMediaLoader,
mediaSource = MediaSource(
url = MXC_URI,
url = mxcUri,
json = null
),
mimeType = "audio/ogg",
@ -133,6 +149,7 @@ private fun createDefaultVoiceMessageMediaRepo( @@ -133,6 +149,7 @@ private fun createDefaultVoiceMessageMediaRepo(
)
private const val MXC_URI = "mxc://matrix.org/1234567890abcdefg"
private const val INVALID_MXC_URI = "notAnMxcUri"
private val TemporaryFolder.cachedFilePath get() = "${this.root.path}/temp/voice/matrix.org/1234567890abcdefg"
private fun TemporaryFolder.createCachedFile() = File(cachedFilePath).apply {
parentFile?.mkdirs()

Loading…
Cancel
Save