Skip to content

Commit

Permalink
Fix file:// URI parsing (#1601)
Browse files Browse the repository at this point in the history
A previous change to fix #1344 to handle # characters in literal no-scheme
paths broke file:// scheme URI parsing by including the entire literal URI in
the file path, which breaks encoded paths (like those with spaces) and URIs
with non-path data (such as query parameters).

This partially reverts the prior fix, so that # is not supported in file://
URIs, requiring the user to either use a no-scheme URI or encode the path,
as # is reserved in URIs for fragment declarations.

Fixes: #1513

Test: FileUriMapperTest#ignoresAfterPathCorrectly
Test: FileUriMapperTest#decodesEncodedPath
  • Loading branch information
TheKeeperOfPie committed Jan 30, 2023
1 parent e04404e commit cf296dd
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
18 changes: 17 additions & 1 deletion coil-base/src/androidTest/java/coil/map/FileUriMapperTest.kt
Expand Up @@ -2,6 +2,7 @@ package coil.map

import android.content.ContentResolver.SCHEME_FILE
import android.content.Context
import android.net.Uri
import androidx.core.net.toUri
import androidx.test.core.app.ApplicationProvider
import coil.request.Options
Expand Down Expand Up @@ -52,6 +53,21 @@ class FileUriMapperTest {
fun parsesPoundCharacterCorrectly() {
val path = "/sdcard/fi#le.jpg"
assertEquals(File(path), mapper.map(path.toUri(), Options(context)))
assertEquals(File(path), mapper.map("file://$path".toUri(), Options(context)))
}

/** Regression test: https://github.com/coil-kt/coil/issues/1513 */
@Test
fun ignoresAfterPathCorrectly() {
val expected = File("/sdcard/file.jpg")
val uri = Uri.parse("$SCHEME_FILE:///sdcard/file.jpg?query=value&query2=value#fragment")
assertEquals(expected, mapper.map(uri, Options(context)))
}

/** Regression test: https://github.com/coil-kt/coil/issues/1513 */
@Test
fun decodesEncodedPath() {
val expected = File("/sdcard/Some File.jpg")
val uri = Uri.parse("$SCHEME_FILE:///sdcard/Some%20File.jpg")
assertEquals(expected, mapper.map(uri, Options(context)))
}
}
9 changes: 7 additions & 2 deletions coil-base/src/main/java/coil/map/FileUriMapper.kt
Expand Up @@ -11,8 +11,13 @@ internal class FileUriMapper : Mapper<Uri, File> {

override fun map(data: Uri, options: Options): File? {
if (!isApplicable(data)) return null
val uri = if (data.scheme == null) data else data.buildUpon().scheme(null).build()
return File(uri.toString())
if (data.scheme == SCHEME_FILE) {
return data.path?.let(::File)
} else {
// If the scheme is not "file", it's null, representing a literal path on disk.
// Assume the entire input, regardless of any reserved characters, is valid.
return File(data.toString())
}
}

private fun isApplicable(data: Uri): Boolean {
Expand Down

0 comments on commit cf296dd

Please sign in to comment.