Fix File.toUrl and DirectoryContainer (#727)

Author: qnga

readium/lcp/src/main/java/org/readium/r2/lcp/license/container/LcplLicenseContainer.kt

@@ -31,7 +31,7 @@ internal class LcplLicenseContainer(private val licenseFile: File) : WritableLic
         try {
             licenseFile.writeBytes(license.toByteArray())
         } catch (e: Exception) {
-            throw LcpException(LcpError.Container.WriteFailed(licenseFile.toUrl()))
+            throw LcpException(LcpError.Container.WriteFailed(licenseFile.toUrl(isDirectory = false)))
         }
     }
 }

readium/shared/src/main/java/org/readium/r2/shared/util/Url.kt

@@ -166,12 +166,16 @@ public sealed class Url : Parcelable {
      * Relativizes the given [url] against this URL.
      *
      * For example:
-     *     this = "http://example.com/foo"
+     *     this = "http://example.com/foo/"
      *     url = "http://example.com/foo/bar/baz"
      *     result = "bar/baz"
      */
-    public open fun relativize(url: Url): Url =
-        checkNotNull(toURI().relativize(url.toURI()).toUrl())
+    public open fun relativize(url: Url): Url {
+        // Unlike the regular JRE (used in unit tests), the Android implementation of URI doesn't
+        // add "/" at the end of the base if it's missing. We might need to align the behaviors
+        // at some point.
+        return checkNotNull(toURI().relativize(url.toURI()).toUrl())
+    }
 
     /**
      * Normalizes the URL using a subset of the RFC-3986 rules.
@@ -365,8 +369,23 @@ public fun Url.Companion.fromLegacyHref(href: String): Url? =
 public fun Url.Companion.fromEpubHref(href: String): Url? =
     Url(href) ?: fromDecodedPath(href)
 
-public fun File.toUrl(): AbsoluteUrl =
-    checkNotNull(AbsoluteUrl(Uri.fromFile(this)))
+/**
+ * Creates a URL pointing to this [File] which must denote an absolute path.
+ *
+ * @param isDirectory If the URL must end with a trailing slash because it points to a directory.
+ */
+public fun File.toUrl(isDirectory: Boolean): AbsoluteUrl {
+    require(isAbsolute)
+
+    val uri = Uri.Builder().also {
+        it.scheme("file")
+        it.authority("")
+        it.path(path)
+        if (isDirectory) it.appendPath("")
+    }.build()
+
+    return checkNotNull(AbsoluteUrl(uri))
+}
 
 public fun Uri.toUrl(): Url? =
     Url(this)

readium/shared/src/main/java/org/readium/r2/shared/util/file/DirectoryContainer.kt

@@ -35,13 +35,13 @@ public class DirectoryContainer(
     public companion object {
 
         public suspend operator fun invoke(root: File): Try<DirectoryContainer, FileSystemError> {
-            val rootUrl = root.toUrl()
+            val rootUrl = root.toUrl(isDirectory = true)
             val entries =
                 try {
                     withContext(Dispatchers.IO) {
                         root.walk()
                             .filter { it.isFile }
-                            .map { rootUrl.relativize(it.toUrl()) }
+                            .map { rootUrl.relativize(it.toUrl(isDirectory = false)) }
                             .toSet()
                     }
                 } catch (e: SecurityException) {

readium/shared/src/main/java/org/readium/r2/shared/util/file/FileResource.kt

@@ -55,7 +55,7 @@ public class FileResource(
                 }
         )
 
-    override val sourceUrl: AbsoluteUrl = file.toUrl()
+    override val sourceUrl: AbsoluteUrl = file.toUrl(isDirectory = false)
 
     public override suspend fun properties(): Try<Resource.Properties, ReadError> {
         return Try.success(properties)

readium/shared/src/main/java/org/readium/r2/shared/util/zip/FileZipContainer.kt

@@ -137,7 +137,7 @@ internal class FileZipContainer(
         }
     }
 
-    override val sourceUrl: AbsoluteUrl = file.toUrl()
+    override val sourceUrl: AbsoluteUrl = file.toUrl(isDirectory = false)
 
     override val entries: Set<Url> =
         tryOrLog { archive.entries().toList() }

readium/shared/src/main/java/org/readium/r2/shared/util/zip/StreamingZipArchiveProvider.kt

@@ -91,7 +91,7 @@ internal class StreamingZipArchiveProvider {
     internal suspend fun openFile(file: File): Container<Resource> = withContext(Dispatchers.IO) {
         val fileChannel = FileChannelAdapter(file, "r")
         val channel = wrapBaseChannel(fileChannel)
-        StreamingZipContainer(ZipFile(channel), file.toUrl())
+        StreamingZipContainer(ZipFile(channel), file.toUrl(isDirectory = false))
     }
 
     private fun wrapBaseChannel(channel: SeekableByteChannel): SeekableByteChannel {

readium/shared/src/test/java/org/readium/r2/shared/util/UrlTest.kt

@@ -321,7 +321,7 @@ class UrlTest {
 
     @Test
     fun fromFile() {
-        assertEquals(AbsoluteUrl(Uri.parse("file:///tmp/test.txt")), File("/tmp/test.txt").toUrl())
+        assertEquals(AbsoluteUrl(Uri.parse("file:///tmp/test.txt")), File("/tmp/test.txt").toUrl(isDirectory = false))
     }
 
     @Test
@@ -332,6 +332,11 @@ class UrlTest {
         )
     }
 
+    @Test
+    fun fromDirectory() {
+        assertEquals(AbsoluteUrl(Uri.parse("file:///tmp/")), File("/tmp").toUrl(isDirectory = true))
+    }
+
     @Test
     fun fromURI() {
         assertEquals(RelativeUrl(Uri.parse("foo/bar")), URI("foo/bar").toUrl())

test-app/src/main/java/org/readium/r2/testapp/domain/Bookshelf.kt

@@ -99,7 +99,7 @@ class Bookshelf(
         retrieverResult: Try<PublicationRetriever.Result, ImportError>,
     ) {
         retrieverResult
-            .map { addBook(it.publication.toUrl(), it.format, it.coverUrl) }
+            .map { addBook(it.publication.toUrl(isDirectory = false), it.format, it.coverUrl) }
             .onSuccess { channel.send(Event.ImportPublicationSuccess) }
             .onFailure { channel.send(Event.ImportPublicationError(it)) }
     }