Omit Cookie downloads header when empty rather than empty string (#6564)

Task/Issue URL:
https://app.asana.com/1/137249556945/project/488551667048375/task/1211033344698349?focus=true

### Description
From #6516, investigated and
found that empty `Cookie` HTTP headers are tripping up Calibre's web
server.
- Empty `Cookie` headers are legal but there are some sparse reports of
web servers failing to parse them
- Rather than an empty `Cookie` header, we can just omit sending it at
all

That fixes the problem for Calibre (and i'm sure a few other services).
Fixes #6516.

Feature flagged so we can revert to the existing behaviour if we notice
problems because of this change.

### Steps to test this PR

- [x] Test some downloads still succeed as normal (e.g.,
https://file-examples.com/)
- [ ] [Optional]: 
    - [ ] [install Calibre](https://calibre-ebook.com/download)
    - [ ] Tap the `Connect/share` button
    - [ ] `Start content server`
- [ ] Browse to the URL from the Android device running this fixed
branch, and download the sample book

Co-authored-by: Craig Russell <[email protected]>

Author: CDRussell

downloads/downloads-impl/build.gradle

@@ -86,6 +86,7 @@ dependencies {
     testImplementation 'app.cash.turbine:turbine:_'
     testImplementation Testing.robolectric
     testImplementation project(path: ':common-test')
+    testImplementation project(':feature-toggles-test')
 
     coreLibraryDesugaring Android.tools.desugarJdkLibs
 }

downloads/downloads-impl/src/main/java/com/duckduckgo/downloads/impl/DownloadFileService.kt

@@ -35,7 +35,7 @@ interface DownloadFileService {
     @Streaming
     @GET
     fun downloadFile(
-        @Header("Cookie") cookie: String,
+        @Header("Cookie") cookie: String?,
         @Url urlString: String,
     ): Call<ResponseBody>
 }

downloads/downloads-impl/src/main/java/com/duckduckgo/downloads/impl/UrlFileDownloader.kt

@@ -22,6 +22,7 @@ import com.duckduckgo.downloads.api.DownloadFailReason.ConnectionRefused
 import com.duckduckgo.downloads.api.DownloadFailReason.Other
 import com.duckduckgo.downloads.api.FileDownloader
 import com.duckduckgo.downloads.api.model.DownloadItem
+import com.duckduckgo.downloads.impl.feature.FileDownloadFeature
 import com.duckduckgo.downloads.store.DownloadStatus.STARTED
 import java.io.File
 import javax.inject.Inject
@@ -38,6 +39,7 @@ class UrlFileDownloader @Inject constructor(
     private val downloadFileService: DownloadFileService,
     private val urlFileDownloadCallManager: UrlFileDownloadCallManager,
     private val cookieManagerWrapper: CookieManagerWrapper,
+    private val fileDownloadFeature: FileDownloadFeature,
 ) {
 
     @WorkerThread
@@ -50,7 +52,7 @@ class UrlFileDownloader @Inject constructor(
         val directory = pendingFileDownload.directory
         val call = downloadFileService.downloadFile(
             urlString = url,
-            cookie = cookieManagerWrapper.getCookie(url).orEmpty(),
+            cookie = cookieManagerWrapper.getCookie(url).handleNull(),
         )
         val downloadId = Random.nextLong()
         urlFileDownloadCallManager.add(downloadId, call)
@@ -157,6 +159,17 @@ class UrlFileDownloader @Inject constructor(
         return file
     }
 
+    private fun String?.handleNull(): String? {
+        if (this != null) return this
+
+        // if there are no cookies, we omit sending the cookie header when ff is enabled
+        return if (fileDownloadFeature.omitEmptyCookieHeader().isEnabled()) {
+            null
+        } else {
+            "" // legacy behavior, we send an empty cookie header
+        }
+    }
+
     /**
      * This method calculates fake progress that will be used in cases where the file content length is not known.
      * The fake progress curve follows 1-Math.exp(-step)

downloads/downloads-impl/src/main/java/com/duckduckgo/downloads/impl/feature/FileDownloadFeature.kt

@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2025 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.downloads.impl.feature
+
+import com.duckduckgo.anvil.annotations.ContributesRemoteFeature
+import com.duckduckgo.di.scopes.AppScope
+import com.duckduckgo.feature.toggles.api.Toggle
+import com.duckduckgo.feature.toggles.api.Toggle.DefaultFeatureValue
+
+/**
+ * This is the class that represents file download feature flags and kill switches
+ */
+@ContributesRemoteFeature(
+    scope = AppScope::class,
+    featureName = "androidFileDownloadFeature",
+)
+interface FileDownloadFeature {
+
+    // self() is required, but it is not currently used in the codebase
+    @Toggle.DefaultValue(DefaultFeatureValue.TRUE)
+    fun self(): Toggle
+
+    // This kill switch can be used to revert to the old behaviour of sending an empty Cookie header instead of omitting it
+    @Toggle.DefaultValue(DefaultFeatureValue.TRUE)
+    fun omitEmptyCookieHeader(): Toggle
+}

downloads/downloads-impl/src/test/java/com/duckduckgo/downloads/impl/UrlFileDownloaderTest.kt

@@ -16,9 +16,13 @@
 
 package com.duckduckgo.downloads.impl
 
+import android.annotation.SuppressLint
 import com.duckduckgo.common.test.CoroutineTestRule
 import com.duckduckgo.downloads.api.DownloadFailReason
 import com.duckduckgo.downloads.api.FileDownloader
+import com.duckduckgo.downloads.impl.feature.FileDownloadFeature
+import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
+import com.duckduckgo.feature.toggles.api.Toggle
 import java.io.File
 import kotlinx.coroutines.test.runTest
 import okhttp3.ResponseBody
@@ -31,6 +35,7 @@ import org.mockito.kotlin.*
 import retrofit2.Call
 import retrofit2.Response
 
+@SuppressLint("DenyListedApi")
 class UrlFileDownloaderTest {
     @get:Rule
     @Suppress("unused")
@@ -42,15 +47,18 @@ class UrlFileDownloaderTest {
 
     private lateinit var urlFileDownloader: UrlFileDownloader
 
+    private val fileDownloadFeature = FakeFeatureToggleFactory.create(FileDownloadFeature::class.java)
+
     @Before
     fun setup() {
         realFileDownloadManager = RealUrlFileDownloadCallManager()
-        whenever(downloadFileService.downloadFile(anyString(), anyString())).thenReturn(call)
+        whenever(downloadFileService.downloadFile(anyOrNull(), anyString())).thenReturn(call)
 
         urlFileDownloader = UrlFileDownloader(
             downloadFileService,
             realFileDownloadManager,
             FakeCookieManagerWrapper(),
+            fileDownloadFeature = fileDownloadFeature,
         )
     }
 
@@ -171,6 +179,60 @@ class UrlFileDownloaderTest {
         verify(downloadCallback, never()).onError(eq(pendingFileDownload.url), any(), eq(DownloadFailReason.ConnectionRefused))
     }
 
+    @Test
+    fun whenFeatureFlagEnabledThenCookieHeaderOmittedForEmptyCookies() = runTest {
+        fileDownloadFeature.omitEmptyCookieHeader().setRawStoredState(Toggle.State(enable = true))
+
+        val pendingFileDownload = buildPendingDownload("https://example.com/file.txt")
+        urlFileDownloader.downloadFile(pendingFileDownload, "file.txt", mock<DownloadCallback>())
+
+        verify(downloadFileService).downloadFile(cookie = eq(null), urlString = any())
+    }
+
+    @Test
+    fun whenFeatureFlagDisabledThenCookieHeaderEmptyStringForEmptyCookies() = runTest {
+        fileDownloadFeature.omitEmptyCookieHeader().setRawStoredState(Toggle.State(enable = false))
+
+        val pendingFileDownload = buildPendingDownload("https://example.com/file.txt")
+        urlFileDownloader.downloadFile(pendingFileDownload, "file.txt", mock<DownloadCallback>())
+
+        verify(downloadFileService).downloadFile(cookie = eq(""), urlString = any())
+    }
+
+    @Test
+    fun whenFeatureFlagEnabledAndCookieNotNullThenCookieHeaderIncluded() = runTest {
+        fileDownloadFeature.omitEmptyCookieHeader().setRawStoredState(Toggle.State(enable = true))
+
+        val urlFileDownloader = UrlFileDownloader(
+            downloadFileService,
+            realFileDownloadManager,
+            FakeCookieManagerWrapper("session=abc123; token=xyz"),
+            fileDownloadFeature = fileDownloadFeature,
+        )
+
+        val pendingFileDownload = buildPendingDownload("https://example.com/file.txt")
+        urlFileDownloader.downloadFile(pendingFileDownload, "file.txt", mock<DownloadCallback>())
+
+        verify(downloadFileService).downloadFile(cookie = eq("session=abc123; token=xyz"), urlString = any())
+    }
+
+    @Test
+    fun whenFeatureFlagDisabledAndCookieNotNullThenCookieHeaderIncluded() = runTest {
+        fileDownloadFeature.omitEmptyCookieHeader().setRawStoredState(Toggle.State(enable = false))
+
+        val urlFileDownloader = UrlFileDownloader(
+            downloadFileService,
+            realFileDownloadManager,
+            FakeCookieManagerWrapper("session=abc123; token=xyz"),
+            fileDownloadFeature = fileDownloadFeature,
+        )
+
+        val pendingFileDownload = buildPendingDownload("https://example.com/file.txt")
+        urlFileDownloader.downloadFile(pendingFileDownload, "file.txt", mock<DownloadCallback>())
+
+        verify(downloadFileService).downloadFile(cookie = eq("session=abc123; token=xyz"), urlString = any())
+    }
+
     private fun buildPendingDownload(
         url: String,
         contentDisposition: String? = null,
@@ -185,9 +247,9 @@ class UrlFileDownloaderTest {
         )
     }
 
-    private class FakeCookieManagerWrapper : CookieManagerWrapper {
+    private class FakeCookieManagerWrapper(private val cookie: String? = null) : CookieManagerWrapper {
         override fun getCookie(url: String): String? {
-            return null
+            return cookie
         }
     }
 }