Fix XHTML sniffing and add a sniffing round with only media types hints (#167)

Author: qnga

readium/shared/r2-shared/src/main/java/org/readium/r2/shared/util/mediatype/MediaType.kt

@@ -446,18 +446,31 @@ class MediaType(
          *  - Heavy Sniffing reads the bytes to perform more advanced sniffing.
          */
         private suspend fun of(content: SnifferContent?, mediaTypes: List<String>, fileExtensions: List<String>, sniffers: List<Sniffer>): MediaType? {
-            // Light sniffing
-            var context = SnifferContext(mediaTypes = mediaTypes, fileExtensions = fileExtensions)
-            for (sniffer in sniffers) {
-                val mediaType = sniffer(context)
-                if (mediaType != null) {
-                    return mediaType
+            // Light sniffing with only media type hints
+            if (mediaTypes.isNotEmpty()) {
+                val context = SnifferContext(mediaTypes = mediaTypes)
+                for (sniffer in sniffers) {
+                    val mediaType = sniffer(context)
+                    if (mediaType != null) {
+                        return mediaType
+                    }
+                }
+            }
+
+            // Light sniffing with both media type hints and file extensions
+            if (fileExtensions.isNotEmpty()) {
+                val context = SnifferContext(mediaTypes = mediaTypes, fileExtensions = fileExtensions)
+                for (sniffer in sniffers) {
+                    val mediaType = sniffer(context)
+                    if (mediaType != null) {
+                        return mediaType
+                    }
                 }
             }
 
             // Heavy sniffing
             if (content != null) {
-                context = SnifferContext(content = content, mediaTypes = mediaTypes, fileExtensions = fileExtensions)
+                val context = SnifferContext(content = content, mediaTypes = mediaTypes, fileExtensions = fileExtensions)
                 for (sniffer in sniffers) {
                     val mediaType = sniffer(context)
                     if (mediaType != null) {
@@ -470,6 +483,7 @@ class MediaType(
             // Note: This is done after the heavy sniffing of the provided [sniffers], because
             // otherwise it will detect JSON, XML or ZIP formats before we have a chance of sniffing
             // their content (for example, for RWPM).
+            val context = SnifferContext(content = content, mediaTypes = mediaTypes, fileExtensions = fileExtensions)
             Sniffers.system(context)?.let { return it }
 
             // If nothing else worked, we try to parse the first valid media type hint.

readium/shared/r2-shared/src/main/java/org/readium/r2/shared/util/mediatype/Sniffer.kt

@@ -33,13 +33,30 @@ object Sniffers {
      * The sniffers order is important, because some formats are subsets of other formats.
      */
     val all: List<Sniffer> = listOf(
-        ::html, ::opds, ::lcpLicense, ::bitmap, ::webpub, ::w3cWPUB, ::epub, ::lpf, ::archive,
+        ::xhtml, ::html, ::opds, ::lcpLicense, ::bitmap, ::webpub, ::w3cWPUB, ::epub, ::lpf, ::archive,
         ::pdf, ::json
     )
 
+    /**
+     * Sniffs an XHTML document.
+     *
+     * Must precede the HTML sniffer.
+     */
+    suspend fun xhtml(context: SnifferContext): MediaType? {
+        if (context.hasFileExtension("xht", "xhtml") || context.hasMediaType("application/xhtml+xml")) {
+            return MediaType.XHTML
+        }
+        context.contentAsXml()?.let {
+            if (it.name.lowercase(Locale.ROOT) == "html" && it.namespace.lowercase(Locale.ROOT).contains("xhtml")) {
+                return MediaType.XHTML
+            }
+        }
+        return null
+    }
+
     /** Sniffs an HTML document. */
     suspend fun html(context: SnifferContext): MediaType? {
-        if (context.hasFileExtension("htm", "html", "xht", "xhtml") || context.hasMediaType("text/html", "application/xhtml+xml")) {
+        if (context.hasFileExtension("htm", "html") || context.hasMediaType("text/html")) {
             return MediaType.HTML
         }
         // [contentAsXml] will fail if the HTML is not a proper XML document, hence the doctype check.

readium/shared/r2-shared/src/main/java/org/readium/r2/shared/util/mediatype/SnifferContext.kt

@@ -32,8 +32,8 @@ import java.util.*
  */
 class SnifferContext internal constructor(
     private val content: SnifferContent? = null,
-    mediaTypes: List<String>,
-    fileExtensions: List<String>
+    mediaTypes: List<String> = emptyList(),
+    fileExtensions: List<String> = emptyList()
 ) {
 
     /** Media type hints. */

readium/shared/r2-shared/src/test/java/org/readium/r2/shared/fetcher/ArchiveFetcherTest.kt

@@ -60,13 +60,13 @@ class ArchiveFetcherTest {
         assertEquals(
             listOf(
                 createLink("/mimetype", null),
-                createLink("/EPUB/cover.xhtml" , "text/html", 259L),
+                createLink("/EPUB/cover.xhtml" , "application/xhtml+xml", 259L),
                 createLink("/EPUB/css/epub.css",  "text/css", 595L),
                 createLink("/EPUB/css/nav.css", "text/css", 306L),
                 createLink("/EPUB/images/cover.png", "image/png", 35809L),
-                createLink("/EPUB/nav.xhtml", "text/html", 2293L),
+                createLink("/EPUB/nav.xhtml", "application/xhtml+xml", 2293L),
                 createLink("/EPUB/package.opf", null, 773L),
-                createLink("/EPUB/s04.xhtml", "text/html", 118269L),
+                createLink("/EPUB/s04.xhtml", "application/xhtml+xml", 118269L),
                 createLink("/EPUB/toc.ncx", null, 1697),
                 createLink("/META-INF/container.xml", "text/xml", 176)
             ),

readium/shared/r2-shared/src/test/java/org/readium/r2/shared/util/mediatype/SnifferTest.kt

@@ -133,13 +133,17 @@ class SnifferTest {
     fun `sniff HTML`() = runBlocking {
         assertEquals(MediaType.HTML, MediaType.of(fileExtension = "htm"))
         assertEquals(MediaType.HTML, MediaType.of(fileExtension = "html"))
-        assertEquals(MediaType.HTML, MediaType.of(fileExtension = "xht"))
-        assertEquals(MediaType.HTML, MediaType.of(fileExtension = "xhtml"))
         assertEquals(MediaType.HTML, MediaType.of(mediaType = "text/html"))
-        assertEquals(MediaType.HTML, MediaType.of(mediaType = "application/xhtml+xml"))
         assertEquals(MediaType.HTML, MediaType.ofFile(fixtures.fileAt("html.unknown")))
         assertEquals(MediaType.HTML, MediaType.ofFile(fixtures.fileAt("html-doctype-case.unknown")))
-        assertEquals(MediaType.HTML, MediaType.ofFile(fixtures.fileAt("xhtml.unknown")))
+    }
+
+    @Test
+    fun `sniff XHTML`() = runBlocking {
+        assertEquals(MediaType.XHTML, MediaType.of(fileExtension = "xht"))
+        assertEquals(MediaType.XHTML, MediaType.of(fileExtension = "xhtml"))
+        assertEquals(MediaType.XHTML, MediaType.of(mediaType = "application/xhtml+xml"))
+        assertEquals(MediaType.XHTML, MediaType.ofFile(fixtures.fileAt("xhtml.unknown")))
     }
 
     @Test