From 81cb166d103f7caaa5135b5a1c66d4e978f3619f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateo=20Guzm=C3=A1n?= Date: Mon, 25 Nov 2024 07:34:51 -0800 Subject: [PATCH] fix(image): [android] cache control headers are being overwritten (#47922) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: While trying to write some test cases for `ReactOkHttpNetworkFetcher`, I found that the cache control headers are not being sent over the network request correctly. These cache control headers are always being overwritten by the rest of the headers hence all the logic to set this custom cache control doesn't seem to be working as expected. As per the [Request headers](https://github.com/square/okhttp/blob/2832a9e532ecbc6f1f84b3a07e8e64b75372789c/okhttp/src/main/kotlin/okhttp3/Request.kt#L256) docs, this seems to be the explanation: > /** Removes all headers on this builder and adds [headers]. */ open fun headers(headers: Headers) = commonHeaders(headers) With the new approach by setting the headers first, we ensure that the cache control headers don't get overwritten but they would get added on top of the passed headers. Notice that currently it seems to be overwriting these headers even if there are not headers passed from the Image component (AKA null/empty). See my reproduction example in the test plan. ## Changelog: [ANDROID] [FIXED] - ReactOkHttpNetworkFetcher – cache control headers getting overwritten by the rest of the headers Pull Request resolved: https://github.com/facebook/react-native/pull/47922 Test Plan: By creating the following component, we log the request headers using `FLog` with the previous and the new approach. See the difference on the outputs below: Component: ```tsx ``` Setting the `headers` last (current approach): ```kt val request = Request.Builder() .cacheControl(cacheControlBuilder.build()) .url(uri.toString()) .headers(headers) .get() .build() FLog.w("RequestHeaders", request.headers.toString()) ``` Output (notice that the Cache-Control header is not present): ```bash RequestHeaders com.facebook.react.uiapp W some-header: some-header-value ``` image --- New approach by setting the `headers` first: ```kt val request = Request.Builder() .headers(headers) .cacheControl(cacheControlBuilder.build()) .url(uri.toString()) .get() .build() FLog.w("RequestHeaders", request.headers.toString()) ``` Output (Cache-Control is present now along with the passed headers): ```bash RequestHeaders com.facebook.react.uiapp W some-header: some-header-value Cache-Control: no-cache, no-store ``` image Reviewed By: rshest Differential Revision: D66446820 Pulled By: javache fbshipit-source-id: 2640ea4b0e678a7aa919b919b0b44bedc0da0af4 --- .../facebook/react/modules/fresco/ReactOkHttpNetworkFetcher.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/fresco/ReactOkHttpNetworkFetcher.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/fresco/ReactOkHttpNetworkFetcher.kt index 2e0bec6a119..6eddbb753c1 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/fresco/ReactOkHttpNetworkFetcher.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/fresco/ReactOkHttpNetworkFetcher.kt @@ -60,9 +60,9 @@ internal class ReactOkHttpNetworkFetcher(private val okHttpClient: OkHttpClient) val headers = OkHttpCompat.getHeadersFromMap(requestHeaders) val request = Request.Builder() + .headers(headers) .cacheControl(cacheControlBuilder.build()) .url(uri.toString()) - .headers(headers) .get() .build() fetchWithRequest(fetchState, callback, request)