feat: Download a transfer or folder with api v2 by sirambd · Pull Request #618 · Infomaniak/android-SwissTransfer

sirambd · 2026-04-01T15:51:56Z

No description provided.

github-actions · 2026-04-01T15:53:23Z

PR Reviewer Guide 🔍

(Review updated until commit `23554ac`)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns Path Traversal: The download path construction in `AppDownloadManager` uses unsanitized user-controlled input (`transfer.displayTitle` and `fileUi.fileName`/`fileUi.path`) when building file system paths. An attacker controlling the transfer metadata could potentially inject path traversal sequences (e.g., "../../../") to write files to arbitrary locations on the device's storage. This should be mitigated by validating or sanitizing the title and filename inputs to remove or escape path separators and parent directory references.
⚡ Recommended focus areas for review Hardcoded UniqueDownloadId The `scheduleWork` method returns `UniqueDownloadId(999)` as a hardcoded constant. If multiple downloads are scheduled concurrently, they will all share the same ID, which could cause confusion in logging or tracking. While the current implementation appears to use the transfer UUID and folder ID for actual work identification, returning a constant value defeats the purpose of having a unique ID type. Consider returning a unique value derived from the work request or transfer identifier. return UniqueDownloadId(999) Incorrect TAG constant The TAG is defined as `AppDownloadManager::javaClass.name` which evaluates to "java.lang.Class" rather than the actual class name. This will cause all Sentry logs to be tagged incorrectly. It should be changed to `AppDownloadManager::class.java.name` or a hardcoded string. private val TAG = AppDownloadManager::javaClass.name Path Traversal Risk The `computeFolderDownloadPathWith` function constructs file paths using `transfer.displayTitle` and `fileUi.path` without sanitizing path traversal sequences (e.g., "../"). If the server returns malicious filenames or transfer titles containing such sequences, the app could write files outside the intended Downloads/SwissTransfer directory. This affects both the MediaStore implementation (API 29+) and the direct File access (API < 29). Validate or sanitize these inputs to prevent directory traversal attacks. fun TransferUi.computeFolderDownloadPathWith(fileUi: FileUi): String { val filePath = fileUi.path?.takeIf { it.contains("/") }?.let { if (it.startsWith("/")) it.substringAfter("/") else it } return "$ROOT_FOLDER_NAME/${this.displayTitle}/${filePath?.substringBeforeLast("/") ?: ""}"

github-actions · 2026-04-01T15:56:24Z

+    private fun hasAvailableSpace(requiredBytes: Long): Boolean {
+        val path = when {
+            Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q -> applicationContext.getExternalFilesDir(null)?.path ?: return true
+            else -> Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS).path
+        }
+        val availableBytes = StatFs(path).availableBytes
+        // Add a safety margin of 10% extra space
+        val requiredWithMargin = (requiredBytes * 110) / 100
+        return availableBytes >= requiredWithMargin
+    }


Suggestion: For API level Q and above, the code incorrectly checks available space in the app's private external directory (getExternalFilesDir) instead of the public Downloads directory where files are actually saved via MediaStore. This can cause incorrect "insufficient space" errors or fail to detect actual lack of space. Use the public Downloads directory path for all API levels. [possible issue, importance: 8]

Suggested change

private fun hasAvailableSpace(requiredBytes: Long): Boolean {

val path = when {

Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q -> applicationContext.getExternalFilesDir(null)?.path ?: return true

else -> Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS).path

}

val availableBytes = StatFs(path).availableBytes

// Add a safety margin of 10% extra space

val requiredWithMargin = (requiredBytes * 110) / 100

return availableBytes >= requiredWithMargin

}

private fun hasAvailableSpace(requiredBytes: Long): Boolean {

val path = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS).path

val availableBytes = StatFs(path).availableBytes

// Add a safety margin of 10% extra space

val requiredWithMargin = (requiredBytes * 110) / 100

return availableBytes >= requiredWithMargin

}

github-actions · 2026-04-02T12:38:21Z

Persistent review updated to latest commit 23554ac

github-actions · 2026-04-02T12:40:48Z

+        output: OutputStream,
+        onDownload: suspend (bytesSentTotal: Long, contentLength: Long?) -> Unit,
+    ) {
+        createHttpClient(HttpClient.okHttpClient).prepareGet(url) {


Suggestion: Reuse a single HttpClient instance instead of creating a new one for each file download. Creating new clients per download wastes resources, causes socket exhaustion under concurrent loads, and bypasses connection pooling benefits. Inject a singleton client or store it as a class property. [possible issue, importance: 6]

Suggested change

createHttpClient(HttpClient.okHttpClient).prepareGet(url) {

// Inject or define as singleton property

private val httpClient = createHttpClient(HttpClient.okHttpClient)

private suspend fun downloadFile(

url: String,

output: OutputStream,

onDownload: suspend (bytesSentTotal: Long, contentLength: Long?) -> Unit,

) {

httpClient.prepareGet(url) {

LunarX · 2026-04-07T08:30:14Z

+    private fun buildDownloadNotification(title: String): Notification {
+        return NotificationCompat.Builder(appContext, ChannelIds.downloadChannelId)
+            .setTicker(title)
+            .setContentTitle(title)
+            .setContentText(appContext.getString(R.string.notificationDownloadSuccessNotificationTitle))
+            .setSmallIcon(defaultSmallIcon)
+            .setColor(notificationIconColor)
+            .setAutoCancel(true)
+            .build()
+    }


Do we not put an intent when the notification is clicked?

Added a content intent in c864e86 that opens the system Downloads folder (DownloadManager.ACTION_VIEW_DOWNLOADS) when the notification is tapped. Also moved the notification ID from a magic number to Ids.DownloadSuccess.

@copilot There's no point in displaying the download view because they'll never end up there, instead, packageManager.getLaunchIntentForPackage would be better here.

Updated in 4bb5644 — now using appContext.packageManager.getLaunchIntentForPackage(appContext.packageName) to open the app when the notification is tapped.

LunarX · 2026-04-07T08:56:38Z

+            return UniqueDownloadId(999)
+        }
+
+        fun downloadStatusFlow(transferId: String, folderId: String?): Flow<DownloadStatus> {


Is this really a folderId or rather a fileId?

It's a folderId

LunarX · 2026-04-08T08:32:14Z

+            }
+        }.getOrElse { exception ->
+            return when (exception) {
+                is CancellationException, is NetworkException -> Result.retry()


Why do we want to retry when there's a CancellationException?

Given that, in a Worker, CancellationExceptions mainly originate from the system, and generally serve to tell us to stop and restart later, and since we don’t perform any jobs that we cancel ourselves, we don’t encounter CancellationExceptions that apply to us and need to be handled, because all our errors are exceptions that provide clear information.

LunarX · 2026-04-08T08:58:38Z

+            return workManager.getWorkInfosFlow(workQuery).transform {
+                val workInfo = it.firstOrNull()
+                if (workInfo == null) {
+                    emit(DownloadStatus.Complete)
+                } else if (workInfo.isPending() && isNetworkAvailableFlow.first().not()) {
+                    emit(DownloadStatus.Paused(DownloadStatus.Paused.Reason.WaitingForNetwork))
+                } else {
+                    emit(workInfo.toDownloadStatus())
+                }
+            }


Why not just use a map instead of a transform?

Looking at the code, I don't see any particular reason; clearly, a map would suffice here.
I'd even say a mapLatest, since we have a suspend function.
Check out commit 6b6f993

Copilot

Pull request overview

This PR adds support for downloading an entire transfer or a folder when using SwissTransfer API v2, shifting those downloads to a WorkManager-based implementation while keeping legacy behavior for API v1.

Changes:

Bump swisstransfer dependency to 7.1.1 and add Ktor network dependency.
Introduce DownloadWorker + AppDownloadManager to download transfers/folders in the background and report progress via notifications.
Update UI/viewmodels and helpers to route API v2 download/preview flows through the new scheduler and add new strings for download notifications.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
gradle/libs.versions.toml	Bumps SwissTransfer library version.
app/build.gradle.kts	Adds Ktor dependency needed for new download implementation.
app/src/main/res/values*/strings.xml	Adds new strings for download notifications and “Feedback” button across locales.
app/src/main/java/com/infomaniak/swisstransfer/ui/utils/TransferUiExt.kt	Adds `displayTitle` helper for transfer display naming.
app/src/main/java/com/infomaniak/swisstransfer/ui/utils/NotificationsUtils.kt	Adds a download notification channel and download success/progress notifications.
app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/filesdetails/FilesDetailsViewModel.kt	Injects `DownloadWorker.Scheduler` and passes it to preview/download flows.
app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/transferdetails/TransferManagerExt.kt	Routes preview URI/status handling based on API version.
app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/transferdetails/TransferDownload.kt	Switches v2 transfer/folder downloads to worker-based scheduling and updates open/delete handling.
app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/transferdetails/TransferDetailsViewModel.kt	Injects scheduler and wires it into preview/download flows.
app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/transferdetails/TransferDetailsScreen.kt	Enables download action in the top app bar for sent transfers.
app/src/main/java/com/infomaniak/swisstransfer/ui/components/transfer/TransferItem.kt	Uses `displayTitle` for transfer list items.
app/src/main/java/com/infomaniak/swisstransfer/services/DownloadWorker.kt	New WorkManager worker + scheduler for v2 downloads with foreground progress notifications.
app/src/main/java/com/infomaniak/swisstransfer/services/AppDownloadManager.kt	New downloader that streams files into Public Downloads (MediaStore / filesystem).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+private suspend fun isFileDeleted(id: UniqueDownloadId, transfer: TransferUi, targetFile: FileUi?): Boolean {
    repeat(2) { attemptIndex ->
        runCatching {
-            return downloadManager.doesFileExist(id).not()
+            return when {
+                transfer.isV1() -> downloadManager.doesFileExist(id).not()
+                targetFile != null -> {
+                    downloadManager.doesFileExist(id).not() && AppDownloadManager.doesFileExists(transfer, targetFile).not()
+                }
+                else -> true
+            }


+    val downloadStatusFlow = when {
+        transfer.isV2() -> downloadWorkerScheduler.downloadStatusFlow(
+            transferId = transfer.uuid,
+            folderId = file.uid,
+        )
+        else -> downloadManager.downloadStatusFlow(uniqueDownloadId)
+    }
+    val uriFlow = downloadStatusFlow.transformLatest { status ->
        if (status !is DownloadStatus.Complete) {
            emit(null)
            awaitCancellation()
        }

-        val uri = downloadManager.uriFor(uniqueDownloadId)
+        val uri = when {
+            transfer.isV2() -> AppDownloadManager.uriFor(transfer, file)
+            else -> downloadManager.uriFor(uniqueDownloadId)
+        }


+            return workManager.getWorkInfosFlow(workQuery).transform {
+                val workInfo = it.firstOrNull()
+                if (workInfo == null) {
+                    emit(DownloadStatus.Complete)
+                } else if (workInfo.isPending() && isNetworkAvailableFlow.first().not()) {
+                    emit(DownloadStatus.Paused(DownloadStatus.Paused.Reason.WaitingForNetwork))
+                } else {
+                    emit(workInfo.toDownloadStatus())
+                }
+            }


+        val collection = MediaStore.Downloads.EXTERNAL_CONTENT_URI
+        val itemUri = resolver.insert(collection, contentValues) ?: return@withContext null
+


sirambd · 2026-04-28T17:26:05Z

+        val createdDate = createdDateTimestamp.toDateFromSeconds().format(FORMAT_DATE_TITLE)
+        val title = title ?: createdDate
+        return title


@copilot Check it out—I've improved this code even further in this commit: 075b16f

+    fun downloadSucceeded(tag: String, title: String) {
+        val notification = buildDownloadNotification(title)
+        notificationManagerCompat.notifyCompat(tag, 1, notification)
+    }


Co-authored-by: sirambd <abdourahamane.boinaidi@infomaniak.com>

Agent-Logs-Url: https://github.com/Infomaniak/android-SwissTransfer/sessions/05dec554-32c4-47cd-b6fd-5808dcc71c9a Co-authored-by: sirambd <28200274+sirambd@users.noreply.github.com>

…n download notification Agent-Logs-Url: https://github.com/Infomaniak/android-SwissTransfer/sessions/51089ddf-2222-42f6-8c5c-2e26cd276222 Co-authored-by: sirambd <28200274+sirambd@users.noreply.github.com>

sonarqubecloud · 2026-04-28T17:55:33Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions Bot assigned sirambd Apr 1, 2026

github-actions Bot added the Review effort 3/5 label Apr 1, 2026

github-actions Bot reviewed Apr 1, 2026

View reviewed changes

github-actions Bot reviewed Apr 2, 2026

View reviewed changes

LunarX requested changes Apr 8, 2026

View reviewed changes

LunarX requested a review from Copilot April 28, 2026 14:08

Copilot started reviewing on behalf of LunarX April 28, 2026 14:09 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

LunarX requested changes Apr 28, 2026

View reviewed changes

Comment thread ...rc/main/java/com/infomaniak/swisstransfer/ui/screen/main/transferdetails/TransferDownload.kt Outdated

sirambd added 19 commits April 28, 2026 17:03

chore: Add displayTitle util for a transfer

41c244b

chore: Update st-kmp to 7.1.1

9af988a

chore: Add DownloadWorker for folder

cc47a33

chore: Use Transfer.DisplayTitle and avoid dupplication

f5deb3b

chore: Add AppDownloadManager to handle downloads

94003d5

feat: Show the download transfer button for api v2

d06c947

feat: Notification channel for download + utils

3da8287

chore: Add ktor dependency for download

6c79d81

feat: Handle download folder or swisstransfer for api v2

a203a66

chore: Add mimetype if needed

c5ab69d

chore: Update the download title

bdabfde

refactor: Remove useless line

5a477d9

fix: Download path folders

d739760

chore: Replace rwt by wt mode

eae336b

chore: Update Core

4416b05

chore: Support all work infos

72ec38f

feat: Support thumbnail folder files since api v2

e6af93d

chore: Add a function to calcule available space from a required size

587c809

chore: Improve download notifications

0d26dc6

sirambd and others added 13 commits April 28, 2026 17:11

chore: Support multiple download notification

240c8a6

refactor: Improve parallel download files

6c5dcc6

chore: Replace while true with isActive

480cd3f

chore: Avoid magic value for clarity

ee23b75

chore: Improve clarity logic

4d59518

chore: Add comment

2853809

chore: Mark file deleted since apiV2

f6b6b38

refactor: Rename isNeedDownloadWorker into needsDownloadWorker

b134aa7

Co-authored-by: sirambd <abdourahamane.boinaidi@infomaniak.com>

docs: Add doc to detail useless else case

9ba1a51

Co-authored-by: sirambd <abdourahamane.boinaidi@infomaniak.com>

refactor: Remove useless isApiV2() check

237ff8e

refactor: Merge both conditions into a single one

c13e4c4

chore: Remove unused string res

d4cfb58

chore: Update Core

cb7ee5f

sirambd force-pushed the download-folder branch from 38d672d to cb7ee5f Compare April 28, 2026 15:40

sirambd added 4 commits April 28, 2026 17:57

chore: Use not deprecated notifyCompat

6229f9f

chore: Update the download notification's ticker text

e23df00

chore: Renaming for better readability

edf84ce

chore: Optimize MediaStore query by selecting only ID column

83d331a

sirambd force-pushed the download-folder branch from 0f2ef1a to 83d331a Compare April 28, 2026 16:40

sirambd added 4 commits April 28, 2026 19:03

chore: Apply review suggestion

6b6f993

chore: Replace apply with let to make sure to return the right flow

2d7ace1

refactor: Rename uriForUnderApi29 into uriForBeforeApi29

b0d063c

chore: Simplify displayTitle logic

075b16f

Copilot started work on behalf of sirambd April 28, 2026 17:26 View session

feat: Add content intent to download success notification

c864e86

Agent-Logs-Url: https://github.com/Infomaniak/android-SwissTransfer/sessions/05dec554-32c4-47cd-b6fd-5808dcc71c9a Co-authored-by: sirambd <28200274+sirambd@users.noreply.github.com>

Copilot finished work on behalf of sirambd April 28, 2026 17:32

Copilot started work on behalf of sirambd April 28, 2026 17:49 View session

fix: Use getLaunchIntentForPackage instead of ACTION_VIEW_DOWNLOADS i…

4bb5644

…n download notification Agent-Logs-Url: https://github.com/Infomaniak/android-SwissTransfer/sessions/51089ddf-2222-42f6-8c5c-2e26cd276222 Co-authored-by: sirambd <28200274+sirambd@users.noreply.github.com>

Copilot finished work on behalf of sirambd April 28, 2026 17:51

-        createHttpClient(HttpClient.okHttpClient).prepareGet(url) {
+        // Inject or define as singleton property
+        private val httpClient = createHttpClient(HttpClient.okHttpClient)
+        private suspend fun downloadFile(
+            url: String,
+            output: OutputStream,
+            onDownload: suspend (bytesSentTotal: Long, contentLength: Long?) -> Unit,
+        ) {
+            httpClient.prepareGet(url) {

		val collection = MediaStore.Downloads.EXTERNAL_CONTENT_URI
		val itemUri = resolver.insert(collection, contentValues) ?: return@withContext null

Conversation

sirambd commented Apr 1, 2026

Uh oh!

github-actions Bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit 23554ac)

Uh oh!

Uh oh!

Uh oh!

github-actions Bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 2, 2026

Uh oh!

Uh oh!

github-actions Bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sirambd Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Apr 28, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Apr 1, 2026 •

edited

Loading

(Review updated until commit `23554ac`)

sirambd Apr 28, 2026 •

edited

Loading