From dcbdff087b7f162dcd9917aad278c7cf2515d8a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Sharma?= <737941+loic-sharma@users.noreply.github.com> Date: Thu, 30 Jun 2022 16:15:20 -0700 Subject: [PATCH] Ignore invalid description property in vswhere.exe JSON output (#106836) The `flutter doctor` command uses `vswhere.exe` to verify the Visual Studio installation. This `vswhere.exe` is known to encode its output incorrectly. This is problematic as the `description` property is localized, and in certain languages this results in invalid JSON due to the incorrect encoding. This change introduces a fallback to our `vswhere.exe` output parsing logic: if parsing JSON fails, remove the `description` property and retry parsing the JSON. This fix was also tested on the outputs provided here: https://github.com/flutter/flutter/issues/106601#issuecomment-1170138123 Addresses https://github.com/flutter/flutter/issues/106601 --- .../lib/src/windows/visual_studio.dart | 45 ++++++++++++-- .../windows/visual_studio_test.dart | 58 +++++++++++++++++-- 2 files changed, 93 insertions(+), 10 deletions(-) diff --git a/packages/flutter_tools/lib/src/windows/visual_studio.dart b/packages/flutter_tools/lib/src/windows/visual_studio.dart index e5bef842594..3d9fe4877ee 100644 --- a/packages/flutter_tools/lib/src/windows/visual_studio.dart +++ b/packages/flutter_tools/lib/src/windows/visual_studio.dart @@ -23,11 +23,16 @@ class VisualStudio { required Logger logger, }) : _platform = platform, _fileSystem = fileSystem, - _processUtils = ProcessUtils(processManager: processManager, logger: logger); + _processUtils = ProcessUtils(processManager: processManager, logger: logger), + _logger = logger; final FileSystem _fileSystem; final Platform _platform; final ProcessUtils _processUtils; + final Logger _logger; + + /// Matches the description property from the vswhere.exe JSON output. + final RegExp _vswhereDescriptionProperty = RegExp(r'\s*"description"\s*:\s*".*"\s*,?'); /// True if Visual Studio installation was found. /// @@ -294,9 +299,8 @@ class VisualStudio { ...requirementArguments, ], encoding: encoding); if (whereResult.exitCode == 0) { - final List> installations = - (json.decode(whereResult.stdout) as List).cast>(); - if (installations.isNotEmpty) { + final List>? installations = _tryDecodeVswhereJson(whereResult.stdout); + if (installations != null && installations.isNotEmpty) { return VswhereDetails.fromJson(validateRequirements, installations[0]); } } @@ -304,12 +308,41 @@ class VisualStudio { // Thrown if vswhere doesn't exist; ignore and return null below. } on ProcessException { // Ignored, return null below. - } on FormatException { - // may be thrown if invalid JSON is returned. } return null; } + List>? _tryDecodeVswhereJson(String vswhereJson) { + List? result; + FormatException? originalError; + try { + // Some versions of vswhere.exe are known to encode their output incorrectly, + // resulting in invalid JSON in the 'description' property when interpreted + // as UTF-8. First, try to decode without any pre-processing. + try { + result = json.decode(vswhereJson) as List; + } on FormatException catch (error) { + // If that fails, remove the 'description' property and try again. + // See: https://github.com/flutter/flutter/issues/106601 + vswhereJson = vswhereJson.replaceFirst(_vswhereDescriptionProperty, ''); + + _logger.printTrace('Failed to decode vswhere.exe JSON output. $error' + 'Retrying after removing the unused description property:\n$vswhereJson'); + + originalError = error; + result = json.decode(vswhereJson) as List; + } + } on FormatException { + // Removing the description property didn't help. + // Report the original decoding error on the unprocessed JSON. + _logger.printWarning('Warning: Unexpected vswhere.exe JSON output. $originalError' + 'To see the full JSON, run flutter doctor -vv.'); + return null; + } + + return result.cast>(); + } + /// Returns the details of the best available version of Visual Studio. /// /// If there's a version that has all the required components, that diff --git a/packages/flutter_tools/test/general.shard/windows/visual_studio_test.dart b/packages/flutter_tools/test/general.shard/windows/visual_studio_test.dart index f63fce5ddb3..6f2aa45411a 100644 --- a/packages/flutter_tools/test/general.shard/windows/visual_studio_test.dart +++ b/packages/flutter_tools/test/general.shard/windows/visual_studio_test.dart @@ -92,6 +92,23 @@ const Map _missingStatusResponse = { }, }; +const String _malformedDescriptionResponse = r''' +[ + { + "installationPath": "C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\Community", + "displayName": "Visual Studio Community 2019", + "description": "This description has too many "quotes", + "installationVersion": "16.2.29306.81", + "isRebootRequired": false, + "isComplete": true, + "isPrerelease": false, + "catalog": { + "productDisplayVersion": "16.2.5" + } + } +] +'''; + // Arguments for a vswhere query to search for an installation with the // requirements. const List _requirements = [ @@ -124,7 +141,7 @@ void setMockVswhereResponse( fileSystem.file(vswherePath).createSync(recursive: true); fileSystem.file(cmakePath).createSync(recursive: true); final String finalResponse = responseOverride - ?? (response != null ? json.encode(>[response]) : ''); + ?? (response != null ? json.encode(>[response]) : '[]'); final List requirementArguments = requiredComponents == null ? [] : ['-requires', ...requiredComponents]; @@ -323,7 +340,7 @@ VisualStudioFixture setUpVisualStudio() { logger: logger, processManager: processManager, ); - return VisualStudioFixture(visualStudio, fileSystem, processManager); + return VisualStudioFixture(visualStudio, fileSystem, processManager, logger); } // Set all vswhere query with the required components return null. @@ -717,7 +734,7 @@ void main() { expect(visualStudio.fullVersion, equals('16.2.29306.81')); }); - testWithoutContext('cmakePath returns null when VS is present but when vswhere returns invalid JSON', () { + testWithoutContext('Warns and returns no installation when VS is present but vswhere returns invalid JSON', () { final VisualStudioFixture fixture = setUpVisualStudio(); final VisualStudio visualStudio = fixture.visualStudio; @@ -729,7 +746,19 @@ void main() { fixture.processManager, ); + expect(visualStudio.isInstalled, isFalse); + expect(visualStudio.isComplete, isFalse); + expect(visualStudio.isLaunchable, isFalse); + expect(visualStudio.isPrerelease, isFalse); + expect(visualStudio.isRebootRequired, isFalse); + expect(visualStudio.hasNecessaryComponents, isFalse); + expect(visualStudio.displayName, isNull); + expect(visualStudio.displayVersion, isNull); + expect(visualStudio.installLocation, isNull); + expect(visualStudio.fullVersion, isNull); expect(visualStudio.cmakePath, isNull); + + expect(fixture.logger.warningText, contains('Warning: Unexpected vswhere.exe JSON output')); }); testWithoutContext('Everything returns good values when VS is present with all components', () { @@ -941,6 +970,26 @@ void main() { expect(visualStudio.cmakeGenerator, equals('Visual Studio 16 2019')); expect(visualStudio.displayVersion, equals('\u{FFFD}')); }); + + testWithoutContext('Ignores malformed JSON in description property', () { + setMockVswhereResponse( + fixture.fileSystem, + fixture.processManager, + _requirements, + ['-version', '16'], + null, + _malformedDescriptionResponse, + ); + + expect(visualStudio.isInstalled, true); + expect(visualStudio.isAtLeastMinimumVersion, true); + expect(visualStudio.hasNecessaryComponents, true); + expect(visualStudio.cmakePath, equals(cmakePath)); + expect(visualStudio.cmakeGenerator, equals('Visual Studio 16 2019')); + expect(visualStudio.displayVersion, equals('16.2.5')); + + expect(fixture.logger.warningText, isEmpty); + }); }); group(VswhereDetails, () { @@ -1037,9 +1086,10 @@ void main() { } class VisualStudioFixture { - VisualStudioFixture(this.visualStudio, this.fileSystem, this.processManager); + VisualStudioFixture(this.visualStudio, this.fileSystem, this.processManager, this.logger); final VisualStudio visualStudio; final FileSystem fileSystem; final FakeProcessManager processManager; + final BufferLogger logger; }