Closes mozilla-mobile#7142: Sanitize url in GeckoViewFetch before dow…

…nload
Amejia481 · Jun 1, 2020 · a98f9e5 · a98f9e5
1 parent 2ac91e3
commit a98f9e5
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 3 deletions.
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
@@ -73,7 +73,8 @@ class GeckoViewFetchClient(
     }
 }
 
-private fun Request.toWebRequest(defaultHeaders: Headers): WebRequest = WebRequest.Builder(url)
+@VisibleForTesting
+internal fun Request.toWebRequest(defaultHeaders: Headers): WebRequest = WebRequest.Builder(url.trim())
     .method(method.name)
     .addHeadersFrom(this, defaultHeaders)
     .addBodyFrom(this)

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
@@ -6,6 +6,7 @@ package mozilla.components.browser.engine.gecko.fetch
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import mozilla.components.concept.fetch.Client
+import mozilla.components.concept.fetch.MutableHeaders
 import mozilla.components.concept.fetch.Request
 import mozilla.components.support.test.any
 import mozilla.components.support.test.eq
@@ -18,6 +19,7 @@ import okhttp3.mockwebserver.MockWebServer
 import okhttp3.mockwebserver.RecordedRequest
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
+import org.junit.Assert.assertFalse
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -287,6 +289,13 @@ class GeckoViewFetchUnitTestCases : FetchTestCases() {
         createNewClient().fetch(Request(""))
     }
 
+    @Test
+    fun toResponseMustTrimTheUrl() {
+        val webRequest = Request(url = "\nhttps://www.gruppoapi.com/ricerca-stazioni-servizio/images/logo-gruppoapi-shared.png\n").toWebRequest(MutableHeaders())
+
+        assertFalse(webRequest.uri.contains("\n"))
+    }
+
     private fun mockRequest(headerMap: Map<String, String>? = null, body: String? = null, method: String = "GET") {
         val server = mock<MockWebServer>()
         whenever(server.url(any())).thenReturn(mock())

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
@@ -75,7 +75,8 @@ class GeckoViewFetchClient(
     }
 }
 
-private fun Request.toWebRequest(defaultHeaders: Headers): WebRequest = WebRequest.Builder(url)
+@VisibleForTesting
+internal fun Request.toWebRequest(defaultHeaders: Headers): WebRequest = WebRequest.Builder(url.trim())
     .method(method.name)
     .addHeadersFrom(this, defaultHeaders)
     .addBodyFrom(this)

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
@@ -6,6 +6,7 @@ package mozilla.components.browser.engine.gecko.fetch
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import mozilla.components.concept.fetch.Client
+import mozilla.components.concept.fetch.MutableHeaders
 import mozilla.components.concept.fetch.Request
 import mozilla.components.concept.fetch.Response
 import mozilla.components.support.test.any
@@ -19,6 +20,7 @@ import okhttp3.mockwebserver.MockWebServer
 import okhttp3.mockwebserver.RecordedRequest
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
+import org.junit.Assert.assertFalse
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -295,6 +297,13 @@ class GeckoViewFetchUnitTestCases : FetchTestCases() {
         assertEquals(Response.SUCCESS, builder.toResponse(isBlobUri = true).status)
     }
 
+    @Test
+    fun toResponseMustTrimTheUrl() {
+        val webRequest = Request(url = "\nhttps://www.gruppoapi.com/ricerca-stazioni-servizio/images/logo-gruppoapi-shared.png\n").toWebRequest(MutableHeaders())
+
+        assertFalse(webRequest.uri.contains("\n"))
+    }
+
     private fun mockRequest(headerMap: Map<String, String>? = null, body: String? = null, method: String = "GET") {
         val server = mock<MockWebServer>()
         whenever(server.url(any())).thenReturn(mock())

diff --git a/...gecko/src/main/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchClient.kt b/...gecko/src/main/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchClient.kt
@@ -73,7 +73,8 @@ class GeckoViewFetchClient(
     }
 }
 
-private fun Request.toWebRequest(defaultHeaders: Headers): WebRequest = WebRequest.Builder(url)
+@VisibleForTesting
+internal fun Request.toWebRequest(defaultHeaders: Headers): WebRequest = WebRequest.Builder(url.trim())
     .method(method.name)
     .addHeadersFrom(this, defaultHeaders)
     .addBodyFrom(this)

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
@@ -6,6 +6,7 @@ package mozilla.components.browser.engine.gecko.fetch
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import mozilla.components.concept.fetch.Client
+import mozilla.components.concept.fetch.MutableHeaders
 import mozilla.components.concept.fetch.Request
 import mozilla.components.support.test.any
 import mozilla.components.support.test.eq
@@ -18,6 +19,7 @@ import okhttp3.mockwebserver.MockWebServer
 import okhttp3.mockwebserver.RecordedRequest
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
+import org.junit.Assert.assertFalse
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -287,6 +289,13 @@ class GeckoViewFetchUnitTestCases : FetchTestCases() {
         createNewClient().fetch(Request(""))
     }
 
+    @Test
+    fun toResponseMustTrimTheUrl() {
+        val webRequest = Request(url = "\nhttps://www.gruppoapi.com/ricerca-stazioni-servizio/images/logo-gruppoapi-shared.png\n").toWebRequest(MutableHeaders())
+
+        assertFalse(webRequest.uri.contains("\n"))
+    }
+
     private fun mockRequest(headerMap: Map<String, String>? = null, body: String? = null, method: String = "GET") {
         val server = mock<MockWebServer>()
         whenever(server.url(any())).thenReturn(mock())

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/buildSrc/src/main/java/Config.kt)
 
+* **browser-engine-gecko**, **browser-engine-gecko-beta**, **browser-engine-gecko-nightly**
+  * Fixed issue [#7142](https://github.com/mozilla-mobile/android-components/issues/7142)
+
 * **browser-engine-gecko-nightly**
   * Added support for [onbeforeunload prompt](https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onbeforeunload)