fix: Duplicate Requests for checking if a url hosts PDF

CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.6.2]
+
+### Fixes
+
+- Replace HEAD / GET request for checking if file is PDF, with WebKit's [DownloadListener](https://developer.android.com/reference/android/webkit/DownloadListener). This makes sure that for non-PDF urls, no extra request is done [RMET-5141](https://outsystemsrd.atlassian.net/browse/RMET-5141) / [RPM-6744](https://outsystemsrd.atlassian.net/browse/RPM-6744) 
+
 ## [1.6.1]
 
 ### Fixes

pom.xml

@@ -6,5 +6,5 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>io.ionic.libs</groupId>
     <artifactId>ioninappbrowser-android</artifactId>
-    <version>1.6.1</version>
+    <version>1.6.2</version>
 </project>

src/main/java/com.outsystems.plugins.inappbrowser/osinappbrowserlib/helpers/OSIABPdfHelper.kt

@@ -3,53 +3,12 @@ package com.outsystems.plugins.inappbrowser.osinappbrowserlib.helpers
 import android.content.Context
 import java.io.File
 import java.io.IOException
-import java.net.HttpURLConnection
 import java.net.URL
 
 object OSIABPdfHelper {
 
-    interface UrlFactory {
-        fun create(url: String): URL
-    }
-
-    private class DefaultUrlFactory : UrlFactory {
-        override fun create(url: String): URL = URL(url)
-    }
-
-    fun isContentTypeApplicationPdf(urlString: String): Boolean {
-        return try {
-            // Try to identify if the URL is a PDF using a HEAD request.
-            // If the server does not implement HEAD correctly or does not return the expected content-type,
-            // fall back to a GET request, since some servers only return the correct type for GET.
-            if (checkPdfByRequest(urlString, method = "HEAD")) {
-                true
-            } else {
-                checkPdfByRequest(urlString, method = "GET")
-            }
-        } catch (_: Exception) {
-            false
-        }
-    }
-
-    fun checkPdfByRequest(urlString: String, method: String, urlFactory: UrlFactory = DefaultUrlFactory()): Boolean {
-        var conn: HttpURLConnection? = null
-        return try {
-            conn = (urlFactory.create(urlString).openConnection() as? HttpURLConnection)
-            conn?.run {
-                instanceFollowRedirects = true
-                requestMethod = method
-                if (method == "GET") {
-                    setRequestProperty("Range", "bytes=0-0")
-                }
-                connect()
-                val type = contentType?.lowercase()
-                val disposition = getHeaderField("Content-Disposition")?.lowercase()
-                type == "application/pdf" ||
-                        (type.isNullOrEmpty() && disposition?.contains(".pdf") == true)
-            } ?: false
-        } finally {
-            conn?.disconnect()
-        }
+    fun isPdf(mimeType: String?, contentDisposition: String?): Boolean {
+        return mimeType == "application/pdf" || contentDisposition?.contains(".pdf") == true
     }
 
     @Throws(IOException::class)

src/main/java/com.outsystems.plugins.inappbrowser/osinappbrowserlib/views/OSIABWebViewActivity.kt

@@ -218,6 +218,7 @@ class OSIABWebViewActivity : AppCompatActivity() {
         enableThirdPartyCookies()
 
         setupWebView()
+
         if (urlToOpen != null) {
             handleLoadUrl(urlToOpen, customHeaders)
             showLoadingScreen()
@@ -253,24 +254,10 @@ class OSIABWebViewActivity : AppCompatActivity() {
     }
 
     private fun handleLoadUrl(url: String, additionalHttpHeaders: Map<String, String>? = null) {
-        lifecycleScope.launch(Dispatchers.IO) {
-            if (OSIABPdfHelper.isContentTypeApplicationPdf(url)) {
-                val pdfFile = try { OSIABPdfHelper.downloadPdfToCache(this@OSIABWebViewActivity, url) } catch (_: IOException) { null }
-                if (pdfFile != null) {
-                    withContext(Dispatchers.Main) {
-                        webView.stopLoading()
-                        originalUrl = url
-                        val pdfJsUrl =
-                            PDF_VIEWER_URL_PREFIX + Uri.encode("file://${pdfFile.absolutePath}")
-                        webView.loadUrl(pdfJsUrl)
-                    }
-                    return@launch
-                }
-            }
-
-            withContext(Dispatchers.Main) {
-                webView.loadUrl(url, additionalHttpHeaders ?: emptyMap())
-            }
+        if (additionalHttpHeaders.isNullOrEmpty()) {
+            webView.loadUrl(url)
+        } else {
+            webView.loadUrl(url, additionalHttpHeaders)
         }
     }
 
@@ -327,6 +314,14 @@ class OSIABWebViewActivity : AppCompatActivity() {
                 options.showURL && options.showToolbar
             )
         webView.webChromeClient = customWebChromeClient()
+
+        webView.setDownloadListener { url, _, contentDisposition, mimeType, _ ->
+            handleWebViewDownload(
+                url = url,
+                mimeType = mimeType,
+                contentDisposition = contentDisposition
+            )
+        }
     }
 
     /**
@@ -346,6 +341,38 @@ class OSIABWebViewActivity : AppCompatActivity() {
         return OSIABWebChromeClient()
     }
 
+    /**
+     * Implement the WebKit DownloadListener and handle downloading and previewing PDF files
+     */
+    private fun handleWebViewDownload(
+        url: String?,
+        mimeType: String?,
+        contentDisposition: String?
+    ) {
+        if (OSIABPdfHelper.isPdf(mimeType, contentDisposition) &&
+            (!url.isNullOrEmpty() && !url.startsWith(PDF_VIEWER_URL_PREFIX))
+        ) {
+            lifecycleScope.launch(Dispatchers.IO) {
+                val pdfFile = try {
+                    OSIABPdfHelper.downloadPdfToCache(this@OSIABWebViewActivity, url)
+                } catch (_: IOException) {
+                    // can happen if we try to press the "save" button in pdf viewer
+                    //  which returns a blob url that we won't be able to download
+                    null
+                }
+                if (pdfFile != null) {
+                    withContext(Dispatchers.Main) {
+                        webView.stopLoading()
+                        originalUrl = url
+                        val pdfJsUrl =
+                            PDF_VIEWER_URL_PREFIX + Uri.encode("file://${pdfFile.absolutePath}")
+                        webView.loadUrl(pdfJsUrl)
+                    }
+                }
+            }
+        }
+    }
+
     /**
      * Handle permission requests
      */
@@ -814,7 +841,7 @@ class OSIABWebViewActivity : AppCompatActivity() {
         if (!showNavigationButtons) {
             navigationView.removeView(nav)
         } else defineNavigationButtons(isLeftRight, content)
-        
+
         if (!showURL) navigationView.removeView(urlText)
         else defineURLView(url, showNavigationButtons, navigationView, toolbarPosition, isLeftRight)
 

src/test/java/com.outsystems.plugins.inappbrowser/osinappbrowserlib/helpers/OSIABPdfHelperTest.kt

@@ -3,175 +3,51 @@ package com.outsystems.plugins.inappbrowser.osinappbrowserlib.helpers
 import android.content.Context
 import io.mockk.every
 import io.mockk.mockk
-import io.mockk.mockkObject
-import io.mockk.unmockkObject
-import io.mockk.verify
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.Test
-import java.net.HttpURLConnection
 import java.net.ServerSocket
 import java.net.Socket
-import java.net.URL
 import java.nio.file.Files
 import kotlin.concurrent.thread
 
 class OSIABPdfHelperTest {
 
-    @Test
-    fun `isContentTypeApplicationPdf returns true if HEAD is PDF`() {
-        mockkObject(OSIABPdfHelper)
-        every { OSIABPdfHelper.checkPdfByRequest(any(), "HEAD", any()) } returns true
-
-        val result = OSIABPdfHelper.isContentTypeApplicationPdf("http://example.com")
-
-        assertTrue(result)
-        verify { OSIABPdfHelper.checkPdfByRequest(any(), "HEAD", any()) }
-        verify(exactly = 0) { OSIABPdfHelper.checkPdfByRequest(any(), "GET", any()) }
-        unmockkObject(OSIABPdfHelper)
-    }
+    // region isPdf
 
     @Test
-    fun `isContentTypeApplicationPdf falls back to GET if HEAD fails`() {
-        mockkObject(OSIABPdfHelper)
-        every { OSIABPdfHelper.checkPdfByRequest(any(), "HEAD", any()) } returns false
-        every { OSIABPdfHelper.checkPdfByRequest(any(), "GET", any()) } returns true
-
-        val result = OSIABPdfHelper.isContentTypeApplicationPdf("http://example.com")
-
-        assertTrue(result)
-        verify { OSIABPdfHelper.checkPdfByRequest(any(), "HEAD", any()) }
-        verify { OSIABPdfHelper.checkPdfByRequest(any(), "GET", any()) }
-        unmockkObject(OSIABPdfHelper)
+    fun `isPdf returns true when mimeType is application pdf`() {
+        assertTrue(OSIABPdfHelper.isPdf("application/pdf", null))
     }
 
     @Test
-    fun `isContentTypeApplicationPdf returns false if both HEAD and GET fail`() {
-        mockkObject(OSIABPdfHelper)
-        every { OSIABPdfHelper.checkPdfByRequest(any(), "HEAD", any()) } returns false
-        every { OSIABPdfHelper.checkPdfByRequest(any(), "GET", any()) } returns false
-
-        val result = OSIABPdfHelper.isContentTypeApplicationPdf("http://example.com")
-
-        assertFalse(result)
-        verify { OSIABPdfHelper.checkPdfByRequest(any(), "HEAD", any()) }
-        verify { OSIABPdfHelper.checkPdfByRequest(any(), "GET", any()) }
-        unmockkObject(OSIABPdfHelper)
+    fun `isPdf returns true when contentDisposition contains pdf extension`() {
+        assertTrue(OSIABPdfHelper.isPdf(null, "attachment; filename=test.pdf"))
     }
 
     @Test
-    fun `isContentTypeApplicationPdf returns false if exception occurs`() {
-        mockkObject(OSIABPdfHelper)
-        every {
-            OSIABPdfHelper.checkPdfByRequest(
-                any(),
-                any(),
-                any()
-            )
-        } throws RuntimeException("Network error")
-
-        val result = OSIABPdfHelper.isContentTypeApplicationPdf("http://example.com")
-
-        assertFalse(result)
-        verify { OSIABPdfHelper.checkPdfByRequest(any(), "HEAD", any()) }
-        unmockkObject(OSIABPdfHelper)
+    fun `isPdf returns true when both mimeType and contentDisposition indicate pdf`() {
+        assertTrue(OSIABPdfHelper.isPdf("application/pdf", "attachment; filename=test.pdf"))
     }
 
     @Test
-    fun `returns true when content type is application_pdf`() {
-        val urlFactory = mockk<OSIABPdfHelper.UrlFactory>()
-        val url = mockk<URL>()
-        val conn = mockk<HttpURLConnection>(relaxed = true)
-        every { urlFactory.create(any()) } returns url
-        every { url.openConnection() } returns conn
-        every { conn.contentType } returns "application/pdf"
-        every { conn.connect() } returns Unit
-
-        val result = OSIABPdfHelper.checkPdfByRequest("http://example.com/test.pdf", "HEAD", urlFactory)
-
-        assertTrue(result)
-        verify { conn.connect() }
-        verify { conn.disconnect() }
+    fun `isPdf returns true when mimeType is not pdf but contentDisposition contains pdf extension`() {
+        assertTrue(OSIABPdfHelper.isPdf("text/html", "attachment; filename=report.pdf"))
     }
 
     @Test
-    fun `returns true when disposition header contains pdf and content type is empty`() {
-        val urlFactory = mockk<OSIABPdfHelper.UrlFactory>()
-        val url = mockk<URL>()
-        val conn = mockk<HttpURLConnection>(relaxed = true)
-        every { urlFactory.create(any()) } returns url
-        every { url.openConnection() } returns conn
-        every { conn.contentType } returns null
-        every { conn.getHeaderField("Content-Disposition") } returns "attachment; filename=test.pdf"
-        every { conn.connect() } returns Unit
-
-        val result = OSIABPdfHelper.checkPdfByRequest("http://example.com/test.pdf", "HEAD", urlFactory)
-
-        assertTrue(result)
-        verify { conn.connect() }
-        verify { conn.disconnect() }
+    fun `isPdf returns false when neither mimeType nor contentDisposition indicate pdf`() {
+        assertFalse(OSIABPdfHelper.isPdf("text/html", "inline"))
     }
 
     @Test
-    fun `returns false when neither content type nor disposition indicate pdf`() {
-        val urlFactory = mockk<OSIABPdfHelper.UrlFactory>()
-        val url = mockk<URL>()
-        val conn = mockk<HttpURLConnection>(relaxed = true)
-        every { urlFactory.create(any()) } returns url
-        every { url.openConnection() } returns conn
-        every { conn.contentType } returns "text/html"
-        every { conn.getHeaderField("Content-Disposition") } returns "inline"
-        every { conn.connect() } returns Unit
-
-        val result = OSIABPdfHelper.checkPdfByRequest("http://example.com/test.pdf", "HEAD", urlFactory)
-
-        assertFalse(result)
-        verify { conn.connect() }
-        verify { conn.disconnect() }
+    fun `isPdf returns false when both are null`() {
+        assertFalse(OSIABPdfHelper.isPdf(null, null))
     }
 
-    @Test
-    fun `sets Range header for GET method`() {
-        val urlFactory = mockk<OSIABPdfHelper.UrlFactory>()
-        val url = mockk<URL>()
-        val conn = mockk<HttpURLConnection>(relaxed = true)
-        every { urlFactory.create(any()) } returns url
-        every { url.openConnection() } returns conn
-        every { conn.contentType } returns "application/pdf"
-        every { conn.connect() } returns Unit
-
-        OSIABPdfHelper.checkPdfByRequest("http://example.com/test.pdf", "GET", urlFactory)
-
-        verify { conn.setRequestProperty("Range", "bytes=0-0") }
-        verify { conn.connect() }
-        verify { conn.disconnect() }
-    }
-
-    @Test
-    fun `returns false if connection is null`() {
-        val urlFactory = mockk<OSIABPdfHelper.UrlFactory>()
-        val url = mockk<URL>()
-        every { urlFactory.create(any()) } returns url
-        every { url.openConnection() } returns null
+    // endregion
 
-        val result = OSIABPdfHelper.checkPdfByRequest("http://example.com/test.pdf", "HEAD", urlFactory)
-
-        assertFalse(result)
-    }
-
-    @Test
-    fun `returns false if exception is thrown`() {
-        val urlFactory = mockk<OSIABPdfHelper.UrlFactory>()
-        every { urlFactory.create(any()) } throws RuntimeException("Network error")
-
-        val result = try {
-            OSIABPdfHelper.checkPdfByRequest("http://example.com/test.pdf", "HEAD", urlFactory)
-        } catch (_: Exception) {
-            false
-        }
-
-        assertFalse(result)
-    }
+    // region downloadPdfToCache
 
     @Test
     fun `downloadPdfToCache creates file with content`() {
@@ -205,4 +81,6 @@ class OSIABPdfHelperTest {
         file.delete()
         cacheDir.deleteRecursively()
     }
+
+    // endregion
 }