diff --git a/CHANGELOG.md b/CHANGELOG.md index e58d361..9f94b24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -202,33 +202,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the whole generator package (every run just warned and skipped), hiding one unused import and two lint issues in its own test suite. -### Known Issues - -- **When `DartWorker.timeoutMs` fires, no terminal event reaches - `NativeWorkManager.events` — and the late result from the abandoned - callback, once it does finish, is dropped too — reproduced on both - Android and iOS.** Found auditing this release's stress suite - (`issue_30 stress` in `stress_and_system_test.dart`): its 12-case matrix - shows a perfect correlation — every case where `delayMs < timeoutMs` - (natural completion, no timeout race) delivered its terminal event; - every case where `timeoutMs` was reached first delivered nothing, ever. - Isolating a single `DartWorker(timeoutMs: 1000, input: {delayMs: 2000})` - confirmed it directly: only the `isStarted` event arrived in a 15 s - window on either platform — nothing at the 1 s timeout mark, and - nothing when the callback's own 2 s delay separately elapsed and it - returned a real result to an invocation nobody was listening for - anymore. Worker completions with **no** timeout race are not implicated - by this — only the timeout-fires case. Confirmed pre-existing on `main` - at v1.6.1 (unaffected by anything in this release) via a worktree - comparison, so it does not block this release, but a real app awaiting - that event on a task that times out would hang indefinitely. The - existing `issue_30 stress` test does not catch this: it treats "no - event arrived" and "correctly failed" as the same outcome (`catch (_) { - actuals.add(0) }`), so 3 of its 4 timeout-should-fire cases pass by - coincidence rather than verifying a failure event was actually - received; only the 4th (`#10`) surfaces at all, and only as a flaky, - timing-order-dependent unhandled-`Future` error rather than a real - assertion failure. Needs its own investigation — not attempted here. +### Test infrastructure + +- **`stress_and_system_test.dart`'s `issue_30 stress` case had a wait-budget + bug that looked, from the outside, exactly like a real "timeoutMs drops + the terminal event" product bug** — enough that an earlier draft of this + entry claimed exactly that before the real cause was traced down. A + `DartWorker` whose `timeoutMs` fires returns a *retryable* failure by + design (matching #46/#47's "`return false` retries" behavior), and the + test never set `maxRetries: 0`. With the default of 3 retries and each + platform's default backoff (Android: WorkManager's own; iOS: 30 s + initial, exponential), a timed-out case doesn't reach a terminal + `WorkInfo` state until all retries are exhausted — which routinely + exceeds the test's own wait budget. That is not a dropped event: + isolating a single `DartWorker(timeoutMs: 1000, delayMs: 2000)` with + `maxRetries: 0` delivers its terminal event at ~1020 ms, exactly at the + timeout mark, confirmed on both platforms. Fixed by adding + `maxRetries: 0` to the enqueue calls (matching what the test actually + intends to measure) and replacing the silent `catch (_) { actuals.add(0) + }` — which made "no event ever arrived" and "correctly failed" read as + the same outcome — with an explicit `expect(neverArrived, isEmpty)` that + names the case if a real dropped-event regression ever does occur. ## [1.6.1] - 2026-09-07 diff --git a/README.md b/README.md index ac8de9b..6867d1b 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ No boilerplate. No native code to write. No `AndroidManifest.xml` changes. Each ```yaml dependencies: - native_workmanager: ^1.7.0 + native_workmanager: ^1.8.0 ``` **2. Initialize once in `main()`:** diff --git a/doc/ANDROID_SETUP.md b/doc/ANDROID_SETUP.md index deda4f5..f073f99 100644 --- a/doc/ANDROID_SETUP.md +++ b/doc/ANDROID_SETUP.md @@ -119,7 +119,7 @@ Add to your `pubspec.yaml`: ```yaml dependencies: - native_workmanager: ^1.7.0 + native_workmanager: ^1.8.0 ``` Run: diff --git a/doc/GETTING_STARTED.md b/doc/GETTING_STARTED.md index 206648f..4f637df 100644 --- a/doc/GETTING_STARTED.md +++ b/doc/GETTING_STARTED.md @@ -34,7 +34,7 @@ Or manually: ```yaml dependencies: - native_workmanager: ^1.7.0 + native_workmanager: ^1.8.0 ``` Then run: diff --git a/doc/MIGRATION_GUIDE.md b/doc/MIGRATION_GUIDE.md index 1570af6..f13fbcc 100644 --- a/doc/MIGRATION_GUIDE.md +++ b/doc/MIGRATION_GUIDE.md @@ -152,7 +152,7 @@ dependencies: **After:** ```yaml dependencies: - native_workmanager: ^1.7.0 + native_workmanager: ^1.8.0 ``` **Then run:** @@ -865,7 +865,7 @@ Use this checklist to track your migration progress: ```yaml dependencies: workmanager: ^0.5.0 - native_workmanager: ^1.7.0 + native_workmanager: ^1.8.0 ``` Migrate tasks one at a time, then remove workmanager when done. diff --git a/doc/MIGRATION_TOOL_README.md b/doc/MIGRATION_TOOL_README.md index 5f7466c..a45c13d 100644 --- a/doc/MIGRATION_TOOL_README.md +++ b/doc/MIGRATION_TOOL_README.md @@ -142,7 +142,7 @@ Updated dependencies file: dependencies: flutter: sdk: flutter - native_workmanager: ^1.7.0 # Replaced workmanager + native_workmanager: ^1.8.0 # Replaced workmanager ``` **Usage:** diff --git a/example/integration_test/issue_66_69_ftl_test.dart b/example/integration_test/issue_66_69_ftl_test.dart index 7b12a39..98f57de 100644 --- a/example/integration_test/issue_66_69_ftl_test.dart +++ b/example/integration_test/issue_66_69_ftl_test.dart @@ -101,28 +101,34 @@ void main() { expect( counterFile.existsSync(), isTrue, - reason: 'issue_66: the callback must have started and written at ' + reason: + 'issue_66: the callback must have started and written at ' 'least one iteration before being cancelled', ); - final iterationsAtCancel = - int.parse(counterFile.readAsStringSync().trim()); + final iterationsAtCancel = int.parse( + counterFile.readAsStringSync().trim(), + ); expect( iterationsAtCancel, lessThan(20), - reason: 'issue_66: cancelling ~600ms in must stop the callback ' + reason: + 'issue_66: cancelling ~600ms in must stop the callback ' 'well short of all 50 iterations — a count this high means ' 'isTaskCancelled() never observed the cancellation', ); - final iterationsAfterWait = - int.parse(counterFile.readAsStringSync().trim()); + final iterationsAfterWait = int.parse( + counterFile.readAsStringSync().trim(), + ); await Future.delayed(const Duration(seconds: 2)); - final iterationsStillAfterWait = - int.parse(counterFile.readAsStringSync().trim()); + final iterationsStillAfterWait = int.parse( + counterFile.readAsStringSync().trim(), + ); expect( iterationsStillAfterWait, equals(iterationsAfterWait), - reason: 'issue_66: iteration count must not still be climbing ' + reason: + 'issue_66: iteration count must not still be climbing ' '2s later — the callback should have returned, not kept ' 'working', ); @@ -161,7 +167,8 @@ void main() { expect( File(savePath).existsSync(), isFalse, - reason: 'issue_69: a cancelled background-session download must ' + reason: + 'issue_69: a cancelled background-session download must ' 'not still write its destination file', ); }, diff --git a/example/integration_test/stress_and_system_test.dart b/example/integration_test/stress_and_system_test.dart index 0df86ba..c93df03 100644 --- a/example/integration_test/stress_and_system_test.dart +++ b/example/integration_test/stress_and_system_test.dart @@ -228,6 +228,21 @@ void main() { // a single 25 s ceiling regardless of timeoutMs, so a single broken path // would silently fail all of them. This test runs heterogeneous timeouts // concurrently so a regression manifests as wrong-bucket completion. + // + // maxRetries: 0 below is load-bearing, not decoration. A DartWorker that + // times out returns a retryable failure by design (matching #46/#47's + // "return false retries" fix) — without capping retries, a timed-out case + // doesn't reach a terminal WorkInfo state until all default-3 retries with + // growing backoff are exhausted (Android: WorkManager's own backoff; + // iOS: 30 s initial delay, exponential), which blows well past `waitMs` + // below. That is not a dropped event — confirmed directly: an isolated + // DartWorker(timeoutMs: 1000, delayMs: 2000) with maxRetries: 0 delivers + // its terminal event at ~1020 ms, exactly at the timeout mark, on both + // platforms. Previously this test had no maxRetries override, so its own + // wait budget was racing the retry backoff rather than the timeout itself + // — an early, misleadingly convenient draft of a "fixed the timeoutMs + // event bug" CHANGELOG entry was written from that race before this was + // traced to its actual cause and corrected. testWidgets( 'issue_30 stress: 12 concurrent DartWorkers honor per-task timeoutMs', (tester) async { @@ -274,18 +289,40 @@ void main() { input: {'delayMs': delayMs, 'tag': '#$i'}, timeoutMs: timeoutMs, ), + constraints: const Constraints(maxRetries: 0), ); } + // With maxRetries: 0 above, every case — success or timeout-induced + // failure — reaches a terminal WorkInfo state near its own budget, so + // waitMs genuinely timing out here means a terminal event never + // arrived at all. That used to be silently folded into "0 = fail", + // indistinguishable from a real, correctly-delivered failure event — + // which is exactly how a real dropped-event regression would slip + // through this test undetected. Fail loudly instead: name which case + // never got an event, so a future regression reads as a real + // assertion failure, not a coincidental match against `expected`. final actuals = []; + final neverArrived = []; for (var i = 0; i < waits.length; i++) { try { final event = await waits[i]; actuals.add(event.success ? 1 : 0); } catch (_) { - actuals.add(0); // timed out waiting → treat as fail + neverArrived.add(i); + actuals.add(-1); // sentinel — never equals expected 0 or 1 } } + expect( + neverArrived, + isEmpty, + reason: + 'No terminal event ever arrived for case(s) $neverArrived ' + '(delay/timeoutMs: ${neverArrived.map((i) => cases[i]).toList()}) ' + 'within their wait budget. With maxRetries: 0 this is not a slow ' + 'retry — either NativeWorkManager.events dropped the event, or ' + "the worker's own execution hung past timeoutMs.", + ); final expected = cases.map((c) => c[2]).toList(); // Print on mismatch so simulator output points at the wrong case. diff --git a/ios/Tests/SecurityValidatorTests.swift b/ios/Tests/SecurityValidatorTests.swift index 6076922..45a3db4 100644 --- a/ios/Tests/SecurityValidatorTests.swift +++ b/ios/Tests/SecurityValidatorTests.swift @@ -189,4 +189,62 @@ class SecurityValidatorTests: XCTestCase { XCTAssertTrue(logged.contains("Content-Type"), "Non-sensitive headers should be logged") } + + // MARK: - sanitizedURL + + // sanitizedURL redacted the query string but never touched RFC 3986 UserInfo, so a URL + // carrying its credentials in the authority (https://user:pass@host/...) printed the + // password verbatim into logs and persisted WorkerResult failure messages. Same bug shape + // kmpworkmanager's own (unrelated) SecurityValidator.sanitizedURL had just been fixed for — + // found by comparison, not shared code. Cases mirror the Android regression test + // (SecurityValidatorSanitizedUrlTest.kt) and the standalone Swift script this fix was + // originally verified against before being folded into this file. + + func testSanitizedURL_RedactsUserInfoCredentials() { + let sanitized = SecurityValidator.sanitizedURL("https://admin:secret123@api.example.com/data") + XCTAssertFalse(sanitized.contains("secret123"), "password must not appear in sanitized output") + XCTAssertFalse(sanitized.contains("admin"), "username must not appear in sanitized output") + XCTAssertTrue(sanitized.contains("[REDACTED]@")) + XCTAssertTrue(sanitized.contains("api.example.com/data")) + } + + func testSanitizedURL_RedactsUserInfoAndQueryTogether() { + let sanitized = SecurityValidator.sanitizedURL( + "https://admin:secret123@api.example.com/data?token=abc123") + XCTAssertFalse(sanitized.contains("secret123")) + XCTAssertFalse(sanitized.contains("abc123")) + XCTAssertTrue(sanitized.contains("[REDACTED]@")) + } + + func testSanitizedURL_PortAndPathSurviveUserInfoRedaction() { + let sanitized = SecurityValidator.sanitizedURL( + "https://user:pass@api.example.com:8080/path/to/resource") + XCTAssertFalse(sanitized.contains("user:pass")) + XCTAssertTrue(sanitized.contains("api.example.com:8080/path/to/resource")) + } + + func testSanitizedURL_NoCredentials_unchanged() { + let url = "https://api.example.com/data" + XCTAssertEqual(SecurityValidator.sanitizedURL(url), url) + } + + func testSanitizedURL_QueryOnly_stillRedactsQueryAsBefore() { + let sanitized = SecurityValidator.sanitizedURL("https://api.example.com/data?token=abc123") + XCTAssertFalse(sanitized.contains("abc123")) + XCTAssertTrue(sanitized.contains("api.example.com/data")) + } + + func testSanitizedURL_AtSignInPath_notAuthority_leftAlone() { + // No "://...@" before the first "/" — the "@" here is just path content. + let url = "https://api.example.com/users/@handle" + XCTAssertEqual(SecurityValidator.sanitizedURL(url), url) + } + + func testSanitizedURL_EmptyString_doesNotCrash() { + // URLComponents(string: "") does not return nil (verified directly, not assumed) — + // it's a valid empty-path components value with no authority and no query, so both + // redactUserInfo's early return (no "://") and the no-query branch apply and this + // comes back unchanged. Pre-existing behavior, unrelated to this fix. + XCTAssertEqual(SecurityValidator.sanitizedURL(""), "") + } }