Apply suggestion:

- Added `MediaSource.safeUrl` property replacing `withCleanUrl` method.
- Made `url` private so it can't be used externally.
- Reverted code in `CoilMediaFetcher`
- Also add tests
This commit is contained in:
Jorge Martín 2026-02-27 09:33:03 +01:00
parent fecbabb0e4
commit 5fb9dcb0da
9 changed files with 50 additions and 41 deletions

View file

@ -9,7 +9,7 @@
package io.element.android.libraries.matrix.api.media
import android.os.Parcelable
import androidx.core.net.toUri
import kotlinx.parcelize.IgnoredOnParcel
import kotlinx.parcelize.Parcelize
@Parcelize
@ -17,31 +17,20 @@ data class MediaSource(
/**
* Url of the media.
*/
val url: String,
private val url: String,
/**
* This is used to hold data for encrypted media.
*/
val json: String? = null,
) : Parcelable
/**
* Returns a new [MediaSource] with a valid URL.
*/
fun MediaSource.withCleanUrl(): MediaSource {
val uri = this.url.toUri()
if (uri.scheme != "mxc") return this
// We've seen some MXC urls in the wild having some `mxc://foo/bar#auto` fragment suffix, which is invalid
val cleanedUrl = buildString {
append(uri.scheme)
if (!this.endsWith("://")) {
append("://")
}
append(uri.host)
if (uri.path != null) {
append(uri.path)
}
) : Parcelable {
/**
* A URL with invalid parts (like `#fragment`, if it's an MXC url) removed.
*/
@IgnoredOnParcel
val safeUrl = if (url.startsWith("mxc")) {
// We've seen some MXC urls in the wild having some `mxc://foo/bar#auto` fragment suffix, which is invalid
url.substringBefore("#")
} else {
url
}
return this.copy(url = cleanedUrl)
}

View file

@ -0,0 +1,26 @@
/*
* Copyright (c) 2026 Element Creations Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.libraries.matrix.api.media
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.matrix.test.media.aMediaSource
import org.junit.Test
class MediaSourceTest {
@Test
fun `safeUrl removes the fragment part in MXC urls`() {
val mediaSource = aMediaSource(url = "mxc://matrix.org/url#fragment")
assertThat(mediaSource.safeUrl).isEqualTo("mxc://matrix.org/url")
}
@Test
fun `safeUrl keeps the fragment part in a non-MXC url`() {
val mediaSource = aMediaSource(url = "https://matrix.org/url#fragment")
assertThat(mediaSource.safeUrl).isEqualTo("https://matrix.org/url#fragment")
}
}