Close mozilla-mobile#8823 allow data urls to be download

using the new gv downloaded api
Amejia481 · Oct 28, 2020 · 0b2dda9 · 0b2dda9
1 parent 1befbad
commit 0b2dda9
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 12 deletions.
diff --git a/...ne-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt b/...ne-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
@@ -797,7 +797,7 @@ class GeckoEngineSession(
                     url = url,
                     mimeType = contentType
                 )
-                val response = webResponse.toResponse(isBlobUri = url.startsWith("blob:"))
+                val response = webResponse.toResponse()
 
                 notifyObservers {
                     onExternalResource(

diff --git a/...-beta/src/main/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchClient.kt b/...-beta/src/main/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchClient.kt
@@ -62,7 +62,7 @@ class GeckoViewFetchClient(
                 fetchFlags += GeckoWebExecutor.FETCH_FLAGS_NO_REDIRECTS
             }
             val webResponse = executor.fetch(webRequest, fetchFlags).poll(readTimeOutMillis)
-            webResponse?.toResponse(request.isBlobUri()) ?: throw IOException("Fetch failed with null response")
+            webResponse?.toResponse() ?: throw IOException("Fetch failed with null response")
         } catch (e: TimeoutException) {
             throw SocketTimeoutException()
         } catch (e: WebRequestError) {
@@ -103,11 +103,13 @@ private fun WebRequest.Builder.addBodyFrom(request: Request): WebRequest.Builder
     return this
 }
 
-internal fun WebResponse.toResponse(isBlobUri: Boolean): Response {
+internal fun WebResponse.toResponse(): Response {
+    val isBlobUri = uri.startsWith("blob:")
+    val isDataUri = uri.startsWith("data:")
     val headers = translateHeaders(this)
-    // We use the same API for blobs and HTTP requests, but blobs won't receive a status code.
+    // We use the same API for blobs,data URLs and HTTP requests, but blobs won't receive a status code.
     // If no exception is thrown we assume success.
-    val status = if (isBlobUri) SUCCESS else statusCode
+    val status = if (isBlobUri || isDataUri) SUCCESS else statusCode
     return Response(
         uri,
         status,

diff --git a/...rc/test/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchUnitTestCases.kt b/...rc/test/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchUnitTestCases.kt
@@ -265,8 +265,13 @@ class GeckoViewFetchUnitTestCases : FetchTestCases() {
     @Test
     fun toResponseMustReturn200ForBlobUrls() {
         val builder = WebResponse.Builder("blob:https://mdn.mozillademos.org/d518464c-5075-9046-aef2-9c313214ed53").statusCode(0).build()
+        assertEquals(Response.SUCCESS, builder.toResponse().status)
+    }
 
-        assertEquals(Response.SUCCESS, builder.toResponse(isBlobUri = true).status)
+    @Test
+    fun toResponseMustReturn200ForDataUrls() {
+        val builder = WebResponse.Builder("data:,Hello%2C%20World!").statusCode(0).build()
+        assertEquals(Response.SUCCESS, builder.toResponse().status)
     }
 
     private fun mockRequest(headerMap: Map<String, String>? = null, body: String? = null, method: String = "GET") {

diff --git a/...gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt b/...gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
@@ -804,7 +804,7 @@ class GeckoEngineSession(
                     url = url,
                     mimeType = contentType
                 )
-                val response = webResponse.toResponse(isBlobUri = url.startsWith("blob:"))
+                val response = webResponse.toResponse()
 
                 notifyObservers {
                     onExternalResource(

diff --git a/...ghtly/src/main/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchClient.kt b/...ghtly/src/main/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchClient.kt
@@ -62,7 +62,7 @@ class GeckoViewFetchClient(
                 fetchFlags += GeckoWebExecutor.FETCH_FLAGS_NO_REDIRECTS
             }
             val webResponse = executor.fetch(webRequest, fetchFlags).poll(readTimeOutMillis)
-            webResponse?.toResponse(request.isBlobUri()) ?: throw IOException("Fetch failed with null response")
+            webResponse?.toResponse() ?: throw IOException("Fetch failed with null response")
         } catch (e: TimeoutException) {
             throw SocketTimeoutException()
         } catch (e: WebRequestError) {
@@ -103,11 +103,13 @@ private fun WebRequest.Builder.addBodyFrom(request: Request): WebRequest.Builder
     return this
 }
 
-internal fun WebResponse.toResponse(isBlobUri: Boolean): Response {
+internal fun WebResponse.toResponse(): Response {
+    val isDataUri = uri.startsWith("data:")
+    val isBlobUri = uri.startsWith("blob:")
     val headers = translateHeaders(this)
-    // We use the same API for blobs and HTTP requests, but blobs won't receive a status code.
+    // We use the same API for blobs, data URLs and HTTP requests, but blobs won't receive a status code.
     // If no exception is thrown we assume success.
-    val status = if (isBlobUri) SUCCESS else statusCode
+    val status = if (isBlobUri || isDataUri) SUCCESS else statusCode
     return Response(
         uri,
         status,

diff --git a/...rc/test/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchUnitTestCases.kt b/...rc/test/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchUnitTestCases.kt
@@ -265,8 +265,13 @@ class GeckoViewFetchUnitTestCases : FetchTestCases() {
     @Test
     fun toResponseMustReturn200ForBlobUrls() {
         val builder = WebResponse.Builder("blob:https://mdn.mozillademos.org/d518464c-5075-9046-aef2-9c313214ed53").statusCode(0).build()
+        assertEquals(Response.SUCCESS, builder.toResponse().status)
+    }
 
-        assertEquals(Response.SUCCESS, builder.toResponse(isBlobUri = true).status)
+    @Test
+    fun toResponseMustReturn200ForDataUrls() {
+        val builder = WebResponse.Builder("data:,Hello%2C%20World!").statusCode(0).build()
+        assertEquals(Response.SUCCESS, builder.toResponse().status)
     }
 
     private fun mockRequest(headerMap: Map<String, String>? = null, body: String? = null, method: String = "GET") {

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -12,6 +12,9 @@ permalink: /changelog/
 * [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt)
 * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/.config.yml)
 
+* **feature-downloads**
+  * 🚒 Bug fixed [issue #8823](https://github.com/mozilla-mobile/android-components/issues/8823) Downloads for data URLs were failing on nightly and beta more details in the [Fenix issue](https://github.com/mozilla-mobile/fenix/issues/16228#issuecomment-717976737).
+
 # 64.0.0
 
 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v63.0.0...v64.0.0)