Skip to content

Commit 4565788

Browse files
fix: QA round bug fixes — CancellationException, HttpDownload resume, CryptoWorker size guard, constraints enforcement tests
- Android: Re-throw CancellationException in 3 Flow-collection coroutines (listenForProgress, listenForEvents, observeWorkCompletion) so structured concurrency propagates correctly - Android: HttpDownloadWorker no longer deletes temp file on network error; partial download is preserved so next retry can use Range header to resume - Android: ImageProcessWorker uses maxOf(1, min(...)) — prevents sampleSize=0 when one dimension already fits, avoids ambiguous min() import - iOS: CryptoWorker validates file size before Data(contentsOf:) to prevent OOM on large files; uses SecRandomCopyBytes for random salt instead of hardcoded string - Integration tests: Add GROUP 9 — DartWorker constraint & delay enforcement, reproducing and verifying fix for issue #1
1 parent 0441f7a commit 4565788

40 files changed

Lines changed: 2408 additions & 1769 deletions

‎CHANGELOG.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
8080

8181
- **Example app: Stale version strings** — updated to v1.0.5
8282

83+
- **Android: `CancellationException` silently swallowed in Flow-collection coroutines** (`NativeWorkmanagerPlugin.kt`)
84+
- **Root cause:** Three `catch (e: Exception)` blocks in `listenForProgress`, `listenForEvents`, and `observeWorkCompletion` caught `kotlinx.coroutines.CancellationException` (a subtype of `Exception`), preventing structured concurrency from propagating coroutine cancellation
85+
- **Impact:** Coroutines leaked when the Flutter plugin was detached (e.g., hot reload, app restart); could cause event-sink callbacks after disposal
86+
- **Fix:** Added `catch (e: kotlinx.coroutines.CancellationException) { throw e }` before the generic `catch (e: Exception)` block in all three locations
87+
88+
- **Android: HttpDownloadWorker deletes temp file on network error, breaking resume** (`HttpDownloadWorker.kt`)
89+
- **Root cause:** The `catch (e: Exception)` block unconditionally called `tempFile.delete()`, destroying the partial download that resume logic depends on
90+
- **Impact:** Resume downloads (`enableResume = true`) always restarted from byte 0 on any network error, wasting bandwidth
91+
- **Fix:** Removed the unconditional temp-file deletion; the file is now preserved so the next retry can use the `Range: bytes=N-` header to resume
92+
93+
- **iOS: `CryptoWorker` loads arbitrarily large files into RAM before size check** (`CryptoWorker.swift`)
94+
- **Root cause:** `Data(contentsOf: inputURL)` was called before `SecurityValidator.validateFileSize()`, so large files caused an OOM crash rather than a clean error
95+
- **Fix:** Moved `validateFileSize()` guard to run before reading the file; added random salt generation (`SecRandomCopyBytes`) replacing the hardcoded string salt, improving encryption security
96+
97+
- **Android: `ImageProcessWorker` uses `min()` without explicit import** (`ImageProcessWorker.kt`)
98+
- **Root cause:** `min(widthRatio, heightRatio)` without an explicit `import kotlin.math.min` could resolve to `java.lang.Math.min` in some Kotlin compiler configurations, producing a compile warning or error
99+
- **Fix:** Changed to `maxOf(1, min(widthRatio, heightRatio))` using Kotlin's built-in `maxOf`; the `max(1, …)` also prevents `sampleSize = 0` when one image dimension already fits within the requested bounds
100+
83101
### Added
84102
- **Device integration test suite** (`example/integration_test/device_integration_test.dart`)
85103
- Covers all trigger types, ExistingPolicy (REPLACE/KEEP), all 11 workers, chains, tags, cancellation, events and progress streams
104+
- **GROUP 9 — DartWorker constraint & delay enforcement** — reproduces and verifies the fix for issue #1: `requiresNetwork` and `initialDelay` are now correctly applied to the WorkManager `WorkRequest` for all task types
86105
- Run with: `flutter test integration_test/device_integration_test.dart --timeout=none`
87106

88107
---
Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,19 @@
11
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
2-
package="dev.brewkits.native_workmanager">
2+
xmlns:tools="http://schemas.android.com/tools"
3+
package="dev.brewkits.native_workmanager">
4+
5+
<!-- Required for isHeavyTask=true (KmpHeavyWorker foreground service) -->
6+
<uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
7+
<!-- Required on Android 14+ (API 34+) for typed foreground services -->
8+
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_DATA_SYNC" />
9+
10+
<application>
11+
<!-- Declare foreground service type for WorkManager's SystemForegroundService.
12+
Required on Android 14+ (API 34+) when using isHeavyTask=true.
13+
If your app needs a different type, override with tools:node="replace". -->
14+
<service
15+
android:name="androidx.work.impl.foreground.SystemForegroundService"
16+
android:foregroundServiceType="dataSync"
17+
tools:node="merge" />
18+
</application>
319
</manifest>

‎android/src/main/kotlin/dev/brewkits/native_workmanager/NativeWorkmanagerPlugin.kt‎

Lines changed: 382 additions & 53 deletions
Large diffs are not rendered by default.

‎android/src/main/kotlin/dev/brewkits/native_workmanager/engine/FlutterEngineManager.kt‎

Lines changed: 29 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package dev.brewkits.native_workmanager.engine
22

33
import android.content.Context
44
import android.util.Log
5+
import io.flutter.FlutterInjector
56
import io.flutter.embedding.engine.FlutterEngine
67
import io.flutter.embedding.engine.dart.DartExecutor
78
import io.flutter.plugin.common.MethodChannel
@@ -187,23 +188,42 @@ object FlutterEngineManager {
187188
// Create engine
188189
val flutterEngine = FlutterEngine(context.applicationContext)
189190

190-
// Start executing Dart callback dispatcher
191-
// Note: Simplified for compatibility - will need proper initialization later
192-
val dartBundlePath = context.assets.toString()
191+
// Get the correct app bundle path for the Flutter assets.
192+
// context.assets.toString() returns the AssetManager object string, which is wrong.
193+
// FlutterLoader provides the correct path for both debug (JIT) and release (AOT) modes.
194+
val bundlePath = FlutterInjector.instance().flutterLoader().findAppBundlePath()
193195

194196
val dartCallback = DartExecutor.DartCallback(
195197
context.assets,
196-
dartBundlePath,
198+
bundlePath,
197199
callbackInfo
198200
)
199201

200-
flutterEngine.dartExecutor.executeDartCallback(dartCallback)
201-
202-
// Create MethodChannel
202+
// Create MethodChannel and register 'dartReady' handler BEFORE starting the Dart isolate.
203+
// This eliminates the race condition where Dart sends 'dartReady' before the Kotlin
204+
// handler is registered, which would cause the message to be dropped and the
205+
// waitForDartReady() 10-second timeout to fire.
203206
val channel = MethodChannel(flutterEngine.dartExecutor.binaryMessenger, CHANNEL_NAME)
207+
val readyDeferred = CompletableDeferred<Unit>()
208+
channel.setMethodCallHandler { call, result ->
209+
when (call.method) {
210+
"dartReady" -> {
211+
Log.d(TAG, "Dart side ready")
212+
readyDeferred.complete(Unit)
213+
result.success(null)
214+
}
215+
else -> result.notImplemented()
216+
}
217+
}
204218

205-
// Wait for Dart side to signal ready
206-
waitForDartReady(channel)
219+
// Now start the Dart isolate — handler is already registered, no race possible.
220+
flutterEngine.dartExecutor.executeDartCallback(dartCallback)
221+
222+
// Wait for Dart to signal it's ready (up to 10 seconds)
223+
withTimeout(10_000L) {
224+
readyDeferred.await()
225+
}
226+
Log.d(TAG, "Dart ready signal received")
207227

208228
// Store references
209229
engine = flutterEngine
@@ -215,33 +235,6 @@ object FlutterEngineManager {
215235
}
216236
}
217237

218-
/**
219-
* Wait for Dart side to signal it's ready.
220-
*
221-
* The Dart callback dispatcher will invoke 'dartReady' method
222-
* to signal that it's initialized and ready to receive tasks.
223-
*/
224-
private suspend fun waitForDartReady(channel: MethodChannel) {
225-
val readyDeferred = CompletableDeferred<Unit>()
226-
227-
channel.setMethodCallHandler { call, result ->
228-
if (call.method == "dartReady") {
229-
Log.d(TAG, "Dart side ready")
230-
readyDeferred.complete(Unit)
231-
result.success(null)
232-
} else {
233-
result.notImplemented()
234-
}
235-
}
236-
237-
// Wait up to 10 seconds for Dart to be ready
238-
withTimeout(10_000L) {
239-
readyDeferred.await()
240-
}
241-
242-
Log.d(TAG, "Dart ready signal received")
243-
}
244-
245238
/**
246239
* Schedule engine disposal check.
247240
*

‎android/src/main/kotlin/dev/brewkits/native_workmanager/workers/CryptoWorker.kt‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import kotlinx.coroutines.Dispatchers
88
import kotlinx.coroutines.withContext
99
import java.io.File
1010
import java.security.MessageDigest
11+
import java.security.SecureRandom
1112
import javax.crypto.Cipher
1213
import javax.crypto.SecretKeyFactory
1314
import javax.crypto.spec.IvParameterSpec
@@ -75,6 +76,7 @@ class CryptoWorker : AndroidWorker {
7576
private const val AES_KEY_SIZE = 256
7677
private const val PBKDF2_ITERATIONS = 100_000
7778
private const val IV_SIZE = 16 // AES block size
79+
private const val SALT_SIZE = 16 // Random salt size
7880
}
7981

8082
data class Config(
@@ -220,8 +222,8 @@ class CryptoWorker : AndroidWorker {
220222
Log.d(TAG, "Encrypting: ${inputFile.name} → ${outputFile.name}")
221223

222224
return try {
223-
// Generate encryption key from password
224-
val salt = "native_workmanager_salt".toByteArray() // In production, use random salt
225+
// Generate random salt (unique per encryption — defeats rainbow tables)
226+
val salt = ByteArray(SALT_SIZE).also { SecureRandom().nextBytes(it) }
225227
val key = generateKey(config.password, salt)
226228

227229
// Encrypt file
@@ -231,7 +233,8 @@ class CryptoWorker : AndroidWorker {
231233

232234
inputFile.inputStream().use { input ->
233235
outputFile.outputStream().use { output ->
234-
// Write IV to output (needed for decryption)
236+
// Write salt + IV to output (both needed for decryption)
237+
output.write(salt)
235238
output.write(iv)
236239

237240
// Encrypt and write data
@@ -307,18 +310,19 @@ class CryptoWorker : AndroidWorker {
307310
Log.d(TAG, "Decrypting: ${inputFile.name} → ${outputFile.name}")
308311

309312
return try {
310-
// Generate decryption key from password
311-
val salt = "native_workmanager_salt".toByteArray() // Must match encryption salt
312-
val key = generateKey(config.password, salt)
313-
314313
inputFile.inputStream().use { input ->
315-
// Read IV from input
314+
// Read salt (first SALT_SIZE bytes), then IV
315+
val salt = ByteArray(SALT_SIZE)
316+
if (input.read(salt) != SALT_SIZE) {
317+
throw IllegalArgumentException("Invalid encrypted file (missing or incomplete salt)")
318+
}
316319
val iv = ByteArray(IV_SIZE)
317-
val ivBytesRead = input.read(iv)
318-
if (ivBytesRead != IV_SIZE) {
320+
if (input.read(iv) != IV_SIZE) {
319321
throw IllegalArgumentException("Invalid encrypted file (missing or incomplete IV)")
320322
}
321323

324+
val key = generateKey(config.password, salt)
325+
322326
// Decrypt file
323327
val cipher = Cipher.getInstance("AES/CBC/PKCS5Padding")
324328
cipher.init(Cipher.DECRYPT_MODE, key, IvParameterSpec(iv))

‎android/src/main/kotlin/dev/brewkits/native_workmanager/workers/DartCallbackWorker.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ class DartCallbackWorkerWrapper(
107107

108108
} catch (e: Exception) {
109109
Log.e(TAG, "Error in DartCallbackWorker", e)
110-
WorkerResult.Failure(e.message ?: "Unknown error", shouldRetry = true)
110+
// shouldRetry = false: emit a definitive failure event instead of retrying forever
111+
WorkerResult.Failure(e.message ?: "Unknown error", shouldRetry = false)
111112
}
112113
}
113114
}

‎android/src/main/kotlin/dev/brewkits/native_workmanager/workers/FileCompressionWorker.kt‎

Lines changed: 32 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import dev.brewkits.kmpworkmanager.background.domain.AndroidWorker
55
import dev.brewkits.kmpworkmanager.background.domain.WorkerResult
66
import dev.brewkits.native_workmanager.workers.utils.ProgressReporter
77
import dev.brewkits.native_workmanager.workers.utils.SecurityValidator
8-
import kotlinx.coroutines.runBlocking
98
import org.json.JSONArray
109
import org.json.JSONObject
1110
import java.io.BufferedInputStream
@@ -143,15 +142,13 @@ class FileCompressionWorker : AndroidWorker {
143142

144143
// Report initial progress
145144
if (taskId != null) {
146-
runBlocking {
147-
ProgressReporter.reportProgress(
148-
taskId = taskId,
149-
progress = 0,
150-
message = "Starting compression...",
151-
currentStep = 0,
152-
totalSteps = totalFiles
153-
)
154-
}
145+
ProgressReporter.reportProgress(
146+
taskId = taskId,
147+
progress = 0,
148+
message = "Starting compression...",
149+
currentStep = 0,
150+
totalSteps = totalFiles
151+
)
155152
}
156153

157154
// ════════════════════════════════════════════════════════════
@@ -183,15 +180,13 @@ class FileCompressionWorker : AndroidWorker {
183180

184181
// Report progress
185182
if (taskId != null) {
186-
runBlocking {
187-
ProgressReporter.reportProgress(
188-
taskId = taskId,
189-
progress = 100,
190-
message = "Compression complete",
191-
currentStep = 1,
192-
totalSteps = 1
193-
)
194-
}
183+
ProgressReporter.reportProgress(
184+
taskId = taskId,
185+
progress = 100,
186+
message = "Compression complete",
187+
currentStep = 1,
188+
totalSteps = 1
189+
)
195190
}
196191
}
197192
}
@@ -201,15 +196,13 @@ class FileCompressionWorker : AndroidWorker {
201196

202197
// Report final progress
203198
if (taskId != null) {
204-
runBlocking {
205-
ProgressReporter.reportProgress(
206-
taskId = taskId,
207-
progress = 100,
208-
message = "Compression complete",
209-
currentStep = filesProcessed.get(),
210-
totalSteps = totalFiles
211-
)
212-
}
199+
ProgressReporter.reportProgress(
200+
taskId = taskId,
201+
progress = 100,
202+
message = "Compression complete",
203+
currentStep = filesProcessed.get(),
204+
totalSteps = totalFiles
205+
)
213206
}
214207

215208
// ════════════════════════════════════════════════════════════
@@ -229,7 +222,9 @@ class FileCompressionWorker : AndroidWorker {
229222

230223
val originalSize = calculateSize(inputFile)
231224
val compressedSize = outputFile.length()
232-
val compressionRatio = (compressedSize.toFloat() / originalSize.toFloat() * 100).toInt()
225+
val compressionRatio = if (originalSize > 0L) {
226+
(compressedSize.toFloat() / originalSize.toFloat() * 100).toInt()
227+
} else 0
233228

234229
Log.d(TAG, "Compression successful:")
235230
Log.d(TAG, " Files: $filesCompressed")
@@ -334,19 +329,16 @@ class FileCompressionWorker : AndroidWorker {
334329
// ✅ FIX #3: Thread-safe increment without lambda capture
335330
val processed = filesProcessedCounter.incrementAndGet()
336331

337-
// Report progress
332+
// Report progress (non-blocking — compressDirectory is not a suspend function)
338333
if (taskId != null && totalFiles > 0) {
339334
val progress = ((processed.toFloat() / totalFiles.toFloat()) * 100).toInt()
340-
341-
runBlocking {
342-
ProgressReporter.reportProgress(
343-
taskId = taskId,
344-
progress = progress,
345-
message = "Compressing: ${file.name}",
346-
currentStep = processed,
347-
totalSteps = totalFiles
348-
)
349-
}
335+
ProgressReporter.reportProgressNonBlocking(
336+
taskId = taskId,
337+
progress = progress,
338+
message = "Compressing: ${file.name}",
339+
currentStep = processed,
340+
totalSteps = totalFiles
341+
)
350342
}
351343
}
352344
}

‎android/src/main/kotlin/dev/brewkits/native_workmanager/workers/FileSystemWorker.kt‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,9 @@ class FileSystemWorker : AndroidWorker {
103103
return WorkerResult.Failure("Destination already exists: $destinationPath (set overwrite=true to replace)")
104104
}
105105

106-
// Path traversal protection
107-
if (!destFile.canonicalPath.startsWith(destFile.parentFile?.canonicalPath ?: "")) {
108-
return WorkerResult.Failure("Path traversal detected in destination")
106+
// Path traversal protection: reject paths containing ..
107+
if (sourcePath.contains("..") || destinationPath.contains("..")) {
108+
return WorkerResult.Failure("Path traversal detected: paths containing '..' are not allowed")
109109
}
110110

111111
return try {
@@ -153,9 +153,9 @@ class FileSystemWorker : AndroidWorker {
153153
return WorkerResult.Failure("Destination already exists: $destinationPath (set overwrite=true to replace)")
154154
}
155155

156-
// Path traversal protection
157-
if (!destFile.canonicalPath.startsWith(destFile.parentFile?.canonicalPath ?: "")) {
158-
return WorkerResult.Failure("Path traversal detected in destination")
156+
// Path traversal protection: reject paths containing ..
157+
if (sourcePath.contains("..") || destinationPath.contains("..")) {
158+
return WorkerResult.Failure("Path traversal detected: paths containing '..' are not allowed")
159159
}
160160

161161
return try {
@@ -328,9 +328,9 @@ class FileSystemWorker : AndroidWorker {
328328
}
329329
}
330330

331-
// Path traversal protection
332-
if (!directory.canonicalPath.startsWith(directory.parentFile?.canonicalPath ?: "")) {
333-
return WorkerResult.Failure("Path traversal detected")
331+
// Path traversal protection: reject paths containing ..
332+
if (path.contains("..")) {
333+
return WorkerResult.Failure("Path traversal detected: paths containing '..' are not allowed")
334334
}
335335

336336
return try {

‎android/src/main/kotlin/dev/brewkits/native_workmanager/workers/HttpDownloadWorker.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,8 @@ class HttpDownloadWorker : AndroidWorker {
290290
}
291291
} catch (e: Exception) {
292292
Log.e(TAG, "Error - ${e.message}", e)
293-
// Clean up temp file on error
294-
tempFile.delete()
293+
// Do NOT delete tempFile — preserve partial download so next retry can resume.
294+
// Only intentional failures (checksum mismatch, invalid path) clean up the temp file.
295295
WorkerResult.Failure(
296296
message = e.message ?: "Unknown error",
297297
shouldRetry = true

0 commit comments

Comments
 (0)