Repository navigation
[flutter_tools] Revert PR: Extract Windows archives using native tar with PowerShell fallback - #193161
Conversation
…h PowerShell fallback (flutter#192298)" This reverts commit b138043.
There was a problem hiding this comment.
Code Review
This pull request replaces external process calls to tar and PowerShell with the Dart archive package for extracting zip and tar archives on Windows, and introduces path validation to prevent Zip Slip vulnerabilities. Feedback on the changes highlights a regression in application_package.dart, where narrowing the caught exception from Exception to ArchiveException will fail to handle file system and format exceptions, potentially causing the tool to crash.
| } on ArchiveException { | ||
| globals.printError('Invalid prebuilt Windows app. Unable to extract from archive.'); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Changing the caught exception type from Exception to ArchiveException introduces a regression:\n\n1. globals.os.unzip reads the file using file.readAsBytesSync(), which throws a FileSystemException (an IOException/Exception, not an ArchiveException) if the file is missing or unreadable.\n2. ZipDecoder().decodeBytes() can throw other standard exceptions like FormatException or TypeError on corrupted archives.\n\nBecause of this change, the unit test 'Bad zipped app, unzip throws exception' in application_package_test.dart had to be deleted because it would now crash the tool instead of failing gracefully.\n\nWe should catch Exception instead of ArchiveException to handle all potential I/O and decoding failures gracefully, and restore the deleted unit test.
| } on ArchiveException { | |
| globals.printError('Invalid prebuilt Windows app. Unable to extract from archive.'); | |
| return null; | |
| } | |
| } on Exception { | |
| globals.printError('Invalid prebuilt Windows app. Unable to extract from archive.'); | |
| return null; | |
| } |
|
autosubmit label was removed for flutter/flutter/193161, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
Fixes #193156
This PR reverts commit b138043, which was causing the Flutter -> Packages and Flutter -> DevTools rolls to fail.
Pre-launch Checklist
///).Text exemption: Revert.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.