[components] Closes https://github.com/mozilla-mobile/android-components/issues/7719: Remove all default headers from GeckoViewFetchClient.

This commit is contained in:
Sebastian Kaspari
2020-09-21 12:05:10 +02:00
parent db7d086612
commit a8b7b69812
14 changed files with 23 additions and 188 deletions

View File

@@ -23,12 +23,6 @@ class GeckoViewFetchTestCases : mozilla.components.tooling.fetch.tests.FetchTest
assertTrue(createNewClient() is GeckoViewFetchClient)
}
@Test
@UiThreadTest
override fun get200WithDefaultHeaders() {
super.get200WithDefaultHeaders()
}
@Test
@UiThreadTest
override fun get200WithGzippedBody() {

View File

@@ -46,7 +46,7 @@ class GeckoViewFetchClient(
return fetchDataUri(request)
}
val webRequest = request.toWebRequest(defaultHeaders)
val webRequest = request.toWebRequest()
val readTimeOut = request.readTimeout ?: maxReadTimeOut
val readTimeOutMillis = readTimeOut.let { (timeout, unit) ->
@@ -75,20 +75,14 @@ class GeckoViewFetchClient(
}
}
private fun Request.toWebRequest(defaultHeaders: Headers): WebRequest = WebRequest.Builder(url)
private fun Request.toWebRequest(): WebRequest = WebRequest.Builder(url)
.method(method.name)
.addHeadersFrom(this, defaultHeaders)
.addHeadersFrom(this)
.addBodyFrom(this)
.cacheMode(if (useCaches) CACHE_MODE_DEFAULT else CACHE_MODE_RELOAD)
.build()
private fun WebRequest.Builder.addHeadersFrom(request: Request, defaultHeaders: Headers): WebRequest.Builder {
defaultHeaders.filter { header ->
request.headers?.contains(header.name) != true
}.forEach { header ->
addHeader(header.name, header.value)
}
private fun WebRequest.Builder.addHeadersFrom(request: Request): WebRequest.Builder {
request.headers?.forEach { header ->
addHeader(header.name, header.value)
}

View File

@@ -69,25 +69,6 @@ class GeckoViewFetchUnitTestCases : FetchTestCases() {
assertTrue(createNewClient() is GeckoViewFetchClient)
}
@Test
override fun get200WithDefaultHeaders() {
val server = mock<MockWebServer>()
whenever(server.url(any())).thenReturn(mock())
val host = server.url("/").host()
val port = server.url("/").port()
val headerMap = mapOf(
"Host" to "$host:$port",
"Accept" to "*/*",
"Accept-Language" to "*/*",
"Accept-Encoding" to "gzip",
"Connection" to "keep-alive",
"User-Agent" to "test")
mockRequest(headerMap)
mockResponse(200)
super.get200WithDefaultHeaders()
}
@Test
override fun get200WithDuplicatedCacheControlRequestHeaders() {
val headerMap = mapOf("Cache-Control" to "no-cache, no-store")

View File

@@ -23,12 +23,6 @@ class GeckoViewFetchTestCases : mozilla.components.tooling.fetch.tests.FetchTest
assertTrue(createNewClient() is GeckoViewFetchClient)
}
@Test
@UiThreadTest
override fun get200WithDefaultHeaders() {
super.get200WithDefaultHeaders()
}
@Test
@UiThreadTest
override fun get200WithGzippedBody() {

View File

@@ -46,7 +46,7 @@ class GeckoViewFetchClient(
return fetchDataUri(request)
}
val webRequest = request.toWebRequest(defaultHeaders)
val webRequest = request.toWebRequest()
val readTimeOut = request.readTimeout ?: maxReadTimeOut
val readTimeOutMillis = readTimeOut.let { (timeout, unit) ->
@@ -75,20 +75,14 @@ class GeckoViewFetchClient(
}
}
private fun Request.toWebRequest(defaultHeaders: Headers): WebRequest = WebRequest.Builder(url)
private fun Request.toWebRequest(): WebRequest = WebRequest.Builder(url)
.method(method.name)
.addHeadersFrom(this, defaultHeaders)
.addHeadersFrom(this)
.addBodyFrom(this)
.cacheMode(if (useCaches) CACHE_MODE_DEFAULT else CACHE_MODE_RELOAD)
.build()
private fun WebRequest.Builder.addHeadersFrom(request: Request, defaultHeaders: Headers): WebRequest.Builder {
defaultHeaders.filter { header ->
request.headers?.contains(header.name) != true
}.forEach { header ->
addHeader(header.name, header.value)
}
private fun WebRequest.Builder.addHeadersFrom(request: Request): WebRequest.Builder {
request.headers?.forEach { header ->
addHeader(header.name, header.value)
}

View File

@@ -69,25 +69,6 @@ class GeckoViewFetchUnitTestCases : FetchTestCases() {
assertTrue(createNewClient() is GeckoViewFetchClient)
}
@Test
override fun get200WithDefaultHeaders() {
val server = mock<MockWebServer>()
whenever(server.url(any())).thenReturn(mock())
val host = server.url("/").host()
val port = server.url("/").port()
val headerMap = mapOf(
"Host" to "$host:$port",
"Accept" to "*/*",
"Accept-Language" to "*/*",
"Accept-Encoding" to "gzip",
"Connection" to "keep-alive",
"User-Agent" to "test")
mockRequest(headerMap)
mockResponse(200)
super.get200WithDefaultHeaders()
}
@Test
override fun get200WithDuplicatedCacheControlRequestHeaders() {
val headerMap = mapOf("Cache-Control" to "no-cache, no-store")

View File

@@ -23,12 +23,6 @@ class GeckoViewFetchTestCases : mozilla.components.tooling.fetch.tests.FetchTest
assertTrue(createNewClient() is GeckoViewFetchClient)
}
@Test
@UiThreadTest
override fun get200WithDefaultHeaders() {
super.get200WithDefaultHeaders()
}
@Test
@UiThreadTest
override fun get200WithGzippedBody() {

View File

@@ -46,7 +46,7 @@ class GeckoViewFetchClient(
return fetchDataUri(request)
}
val webRequest = request.toWebRequest(defaultHeaders)
val webRequest = request.toWebRequest()
val readTimeOut = request.readTimeout ?: maxReadTimeOut
val readTimeOutMillis = readTimeOut.let { (timeout, unit) ->
@@ -75,20 +75,14 @@ class GeckoViewFetchClient(
}
}
private fun Request.toWebRequest(defaultHeaders: Headers): WebRequest = WebRequest.Builder(url)
private fun Request.toWebRequest(): WebRequest = WebRequest.Builder(url)
.method(method.name)
.addHeadersFrom(this, defaultHeaders)
.addHeadersFrom(this)
.addBodyFrom(this)
.cacheMode(if (useCaches) CACHE_MODE_DEFAULT else CACHE_MODE_RELOAD)
.build()
private fun WebRequest.Builder.addHeadersFrom(request: Request, defaultHeaders: Headers): WebRequest.Builder {
defaultHeaders.filter { header ->
request.headers?.contains(header.name) != true
}.forEach { header ->
addHeader(header.name, header.value)
}
private fun WebRequest.Builder.addHeadersFrom(request: Request): WebRequest.Builder {
request.headers?.forEach { header ->
addHeader(header.name, header.value)
}

View File

@@ -69,25 +69,6 @@ class GeckoViewFetchUnitTestCases : FetchTestCases() {
assertTrue(createNewClient() is GeckoViewFetchClient)
}
@Test
override fun get200WithDefaultHeaders() {
val server = mock<MockWebServer>()
whenever(server.url(any())).thenReturn(mock())
val host = server.url("/").host()
val port = server.url("/").port()
val headerMap = mapOf(
"Host" to "$host:$port",
"Accept" to "*/*",
"Accept-Language" to "*/*",
"Accept-Encoding" to "gzip",
"Connection" to "keep-alive",
"User-Agent" to "test")
mockRequest(headerMap)
mockResponse(200)
super.get200WithDefaultHeaders()
}
@Test
override fun get200WithDuplicatedCacheControlRequestHeaders() {
val headerMap = mapOf("Cache-Control" to "no-cache, no-store")

View File

@@ -92,22 +92,6 @@ abstract class Client {
}
}
/**
* List of default headers that should be added to every request unless overridden by the headers in the request.
*/
protected val defaultHeaders: Headers = MutableHeaders(
// Unfortunately some implementations will always send a not removable Accept header. Let's override it with
// a header that accepts everything.
"Accept" to "*/*",
// We expect all clients to implement gzip decoding transparently.
"Accept-Encoding" to "gzip",
// Unfortunately some implementations will always send a not removable Accept-Language header. Let's override
// it with a header that accepts everything.
"Accept-Language" to "*/*"
)
companion object {
const val DATA_URI_BASE64_EXT = ";base64"
const val DATA_URI_SCHEME = "data:"

View File

@@ -7,28 +7,9 @@ package mozilla.components.concept.fetch
import kotlinx.coroutines.async
import kotlinx.coroutines.runBlocking
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Test
class ClientTest {
@Test
fun `Expects gzip encoding default header`() {
val client = TestClient()
val acceptEncoding = client.exposeDefaultHeaders().get("Accept-Encoding")
assertNotNull(acceptEncoding!!)
assertEquals("gzip", acceptEncoding)
}
@Test
fun `Expects accept all default header`() {
val client = TestClient()
val accept = client.exposeDefaultHeaders().get("Accept")
assertNotNull(accept!!)
assertEquals("*/*", accept)
}
@Test
fun `Async request with coroutines`() = runBlocking {
val client = TestClient(responseBody = Response.Body("Hello World".byteInputStream()))
@@ -50,6 +31,4 @@ private class TestClient(
override fun fetch(request: Request): Response {
return Response(responseUrl ?: request.url, responseStatus, responseHeaders, responseBody)
}
fun exposeDefaultHeaders() = defaultHeaders
}

View File

@@ -25,6 +25,11 @@ import java.util.zip.GZIPInputStream
* [HttpURLConnection] implementation of [Client].
*/
class HttpURLConnectionClient : Client() {
private val defaultHeaders: Headers = MutableHeaders(
"User-Agent" to "MozacFetch/${BuildConfig.LIBRARY_VERSION}",
"Accept-Encoding" to "gzip"
)
@Throws(IOException::class)
override fun fetch(request: Request): Response {
if (request.isDataUri()) {
@@ -33,9 +38,8 @@ class HttpURLConnectionClient : Client() {
val connection = (URL(request.url).openConnection() as HttpURLConnection)
connection.setRequestProperty("User-Agent", "MozacFetch/${BuildConfig.LIBRARY_VERSION}")
connection.setupWith(request)
connection.addHeadersFrom(request, defaultHeaders = defaultHeaders)
connection.addHeadersFrom(request, defaultHeaders)
connection.addBodyFrom(request)
return connection.toResponse()

View File

@@ -31,6 +31,11 @@ class OkHttpClient(
private val client: OkHttpClient = OkHttpClient(),
private val context: Context? = null
) : Client() {
private val defaultHeaders: Headers = MutableHeaders(
"User-Agent" to "MozacFetch/${BuildConfig.LIBRARY_VERSION}",
"Accept-Encoding" to "gzip"
)
override fun fetch(request: Request): Response {
if (request.isDataUri()) {
return fetchDataUri(request)
@@ -39,7 +44,6 @@ class OkHttpClient(
val requestClient = client.rebuildFor(request, context)
val requestBuilder = createRequestBuilderWithBody(request)
requestBuilder.addHeader("User-Agent", "MozacFetch/${BuildConfig.LIBRARY_VERSION}")
requestBuilder.addHeadersFrom(request, defaultHeaders = defaultHeaders)
if (!request.useCaches) {

View File

@@ -27,7 +27,6 @@ import java.io.File
import java.io.IOException
import java.lang.Exception
import java.net.SocketTimeoutException
import java.util.Locale
import java.util.UUID
import java.util.concurrent.TimeUnit
@@ -73,48 +72,6 @@ abstract class FetchTestCases {
}
}
@Test
open fun get200WithDefaultHeaders() {
withServerResponding(
MockResponse()
) { client ->
val response = client.fetch(Request(rootUrl()))
assertEquals(200, response.status)
val request = takeRequest()
for (i in 0 until request.headers.size()) {
println(request.headers.name(i) + " = " + request.headers.value(i))
}
val headers = request.headers.filtered()
assertEquals(6, headers.size())
val names = headers.names()
assertTrue(names.contains("Host"))
assertTrue(names.contains("User-Agent"))
assertTrue(names.contains("Connection"))
assertTrue(names.contains("Accept-Encoding"))
assertTrue(names.contains("Accept"))
assertTrue(names.contains("Accept-Language"))
val host = url("/").host()
val port = url("/").port()
assertEquals("$host:$port", request.getHeader("Host"))
assertEquals("*/*", request.getHeader("Accept"))
assertEquals("*/*", request.getHeader("Accept-Language"))
assertEquals("gzip", request.getHeader("Accept-Encoding"))
// Ignoring case here: okhttp uses "Keep-Alive" and httpurlconnection uses "keep-alive".
// I do not want to override the header of either because I do not know if they read it
// internally and require a certain case.
assertEquals("keep-alive", request.getHeader("Connection")
.toLowerCase(Locale.ROOT))
}
}
@Test
open fun get200WithHeaders() {
withServerResponding(