The half-fixed bug: two upstream crashes in dio and flutter/packages

A developer fixed two upstream crashes in the dio HTTP client and Flutter's file_selector_android plugin. In dio, the FusedTransformer crashed on empty response bodies with a custom responseDecoder, while the sibling transformers handled it correctly. In file_selector_android, the app crashed when a selected file couldn't be copied, despite deliberate null-handling code that missed a non-null constraint.

This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry. Both of these bugs were already known before I touched them. One had a linked issue and a fix that covered half the cases. The other had a unit test that asserted the crash, with a comment above it saying // Remove when fixed . They also had the same shape. In both projects the correct behavior already existed elsewhere in the same file: a sibling transformer that handled the input, a sibling branch that reported the failure instead of dying. One path had been left out of it. So neither fix invents behavior. Each one extends a decision the project had already made to the branch that missed it. dio https://github.com/cfug/dio 12.8k stars is the HTTP client most Dart and Flutter apps are built on: interceptors, form data, request cancellation, timeouts, and a pluggable Transformer layer that turns raw response bytes into the typed object your code receives. flutter/packages https://github.com/flutter/packages 5.3k stars is the Flutter team's own monorepo of first-party plugins. The one here is file selector android , the Android implementation behind file selector , the plugin you call when the user needs to pick a file from their device. FormatException on an empty response body FusedTransformer is dio's default transformer. When a request sets a custom responseDecoder and the server returns an empty body with a JSON content type a normal 200 , 204 , or 304 , the transformer's slow path fed the empty string straight into jsonDecode : FormatException: Unexpected end of input at character 1 SyncTransformer and BackgroundTransformer are handed the identical input and both guard the empty string and return it, which dio then normalizes to null for typed JSON responses. Only the default transformer crashes. This was the residual half of 2279 https://github.com/cfug/dio/issues/2279 . PR 2285 https://github.com/cfug/dio/pull/2285 had already fixed the empty-body behavior for the fast path, the one with no custom decoder, but the slow path was never given the same guard. The issue looked closed and the fix looked shipped, and anyone who happened to set a responseDecoder still got an exception. file selector android crashed the app when a selected file couldn't be copied to a readable location: a SecurityException from a content provider that has withdrawn permission, or an IOException on a full disk. FileSelectorApiImpl.toFileResponse looked like it handled this. Abbreviated: String uriPath; FileSelectorNativeException nativeError = null; try { uriPath = FileUtils.getPathFromCopyOfFileFromUri activityPluginBinding.getActivity , uri ; } catch IOException e { // If closing the output stream fails, we cannot be sure that the // target file was written in full. … uriPath = null; } catch SecurityException e { // Calling ContentResolver openInputStream has been reported to throw a // SecurityException on some devices in certain circumstances. Instead of crashing, we // return null . // // See https://github.com/flutter/flutter/issues/100025 for more details. uriPath = null; } catch IllegalArgumentException e { uriPath = FILE SELECTOR EXCEPTION PLACEHOLDER PATH; nativeError = new FileSelectorNativeException / … / ; } return new FileResponse uriPath, contentResolver.getType uri , name, size, bytes, nativeError ; In the two branches that crash, uriPath = null is not an oversight. It is deliberate, and commented as such: "Instead of crashing, we return null." The comment cites The method then builds a FileResponse on that null anyway. path is non-null in the Pigeon-generated FileResponse , so the constructor rejects it at runtime, and the branch written to avoid a crash produces one. The third branch survives because IllegalArgumentException assigns a placeholder path instead of null , so it reaches Dart with a populated nativeError . The file already had a way to report an unreadable file without dying. The two branches that chose null just didn't use it. The crash had also shifted shape over time, which is part of why it survived. flutter/flutter 159568 https://github.com/flutter/flutter/issues/159568 reported it as an IllegalStateException , thrown by the older Java Pigeon generator; since the move to the Kotlin generator, the same failure surfaces as an NPE. The test suite knew about it too. There was a passing test named openFileThrowsNullPointerException whenSecurityExceptionInGetPathFromCopyOfFileFromUri , which asserted the crash: php assertThrows NullPointerException.class, - listenerArgumentCaptor.getValue .onActivityResult 222, Activity.RESULT OK, resultMockIntent ; Above it, a comment: "The behavior is actually an error case and should be fixed … Remove when fixed " So the crash was documented, asserted, and passing CI. file selector android 0.5.2+9The whole fix is one extra condition: if isJsonContent && decodedResponse = null && decodedResponse.isNotEmpty { // slow path decoder, since there was a custom decoder specified return jsonDecode decodedResponse ; } else if customResponseDecoder = null { return decodedResponse; } The obvious patch is a new early return for the empty case. I didn't write one, because the branch that already handles it correctly sits directly underneath: else if customResponseDecoder = null { return decodedResponse; } . Adding isNotEmpty to the guard lets an empty body fall through into it, and dio's existing normalization turns that into null for a typed JSON response. So the fix adds no new code path. It narrows a claim that was too broad: "this string is JSON" becomes "this non-empty string is JSON." A non-empty malformed body still throws, as it did before and as SyncTransformer and BackgroundTransformer do. I wanted the three transformers to agree on this input. Making the default one more forgiving than the other two would have been a different change. The regression test pins the behavior to the specific combination that was broken empty body, JSON content type, custom decoder rather than to the fix: js String decoder List<int bytes, RequestOptions o, ResponseBody rb = utf8.decode bytes, allowMalformed: true ; final response = await FusedTransformer .transformResponse RequestOptions responseType: ResponseType.json, responseDecoder: decoder, , ResponseBody.fromBytes , 200, headers: { Headers.contentTypeHeader: Headers.jsonContentType , } , ; expect response, '' ; Again the correct behavior already existed one level up. The callers of toFileResponse handle a null return: they complete the Dart result with completeWithError "Failed to read file: …" , which is what they do everywhere else a file can't be read. toFileResponse was the only place that pushed on regardless. if uriPath == null { // getPathFromCopyOfFileFromUri can fail to produce a path: either by // throwing handled above by returning a null uriPath or by returning // null directly. Return null so the caller surfaces the failure to Dart, // instead of building a FileResponse with a null path , which the // non-null field rejects at runtime. // See https://github.com/flutter/flutter/issues/159568. return null; } Checking uriPath == null rather than catching the exception at the call site was deliberate. getPathFromCopyOfFileFromUri can also return null without throwing, and that path led to the same crash, so one check covers both. The failure now reaches Dart as a PlatformException you can catch, which is what the uriPath = null branches were aiming for. I inverted the test that asserted the crash rather than deleting it. It now drives onActivityResult for real and asserts that the callback completes with a failure whose message contains "Failed to read file" . I added a second one for the single-file openFile path, which had the same defect and no coverage at all. I also kept the PR scoped to the crash. 159568 https://github.com/flutter/flutter/issues/159568 suggests routing these branches through the typed FileSelectorNativeException mechanism, the way 8184 https://github.com/flutter/packages/pull/8184 did elsewhere. That is the better long-term design, but it needs a Pigeon regeneration and a Dart-facing API change, which would put a breaking change in the same PR as a crash fix. The API cleanup is worth doing on its own. The reflex when you find a crash is to add a check. In both of these the check already existed, in the transformer next door or in the caller one frame up, so most of the work was reading enough of the surrounding code to find it. Once you have, the patch is small enough to review in one sitting. Both bugs had also been looked at before. dio's had an issue and a fix that covered the fast path. The Android one had a test asserting the crash and a comment saying to remove it when fixed. Writing a bug down is not the same as fixing it, and it makes the bug much harder to see the second time around.