Unverified Commit dcbdff08 authored by Loïc Sharma's avatar Loïc Sharma Committed by GitHub

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
parent e34d6bc6
...@@ -23,11 +23,16 @@ class VisualStudio { ...@@ -23,11 +23,16 @@ class VisualStudio {
required Logger logger, required Logger logger,
}) : _platform = platform, }) : _platform = platform,
_fileSystem = fileSystem, _fileSystem = fileSystem,
_processUtils = ProcessUtils(processManager: processManager, logger: logger); _processUtils = ProcessUtils(processManager: processManager, logger: logger),
_logger = logger;
final FileSystem _fileSystem; final FileSystem _fileSystem;
final Platform _platform; final Platform _platform;
final ProcessUtils _processUtils; 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. /// True if Visual Studio installation was found.
/// ///
...@@ -294,9 +299,8 @@ class VisualStudio { ...@@ -294,9 +299,8 @@ class VisualStudio {
...requirementArguments, ...requirementArguments,
], encoding: encoding); ], encoding: encoding);
if (whereResult.exitCode == 0) { if (whereResult.exitCode == 0) {
final List<Map<String, dynamic>> installations = final List<Map<String, dynamic>>? installations = _tryDecodeVswhereJson(whereResult.stdout);
(json.decode(whereResult.stdout) as List<dynamic>).cast<Map<String, dynamic>>(); if (installations != null && installations.isNotEmpty) {
if (installations.isNotEmpty) {
return VswhereDetails.fromJson(validateRequirements, installations[0]); return VswhereDetails.fromJson(validateRequirements, installations[0]);
} }
} }
...@@ -304,12 +308,41 @@ class VisualStudio { ...@@ -304,12 +308,41 @@ class VisualStudio {
// Thrown if vswhere doesn't exist; ignore and return null below. // Thrown if vswhere doesn't exist; ignore and return null below.
} on ProcessException { } on ProcessException {
// Ignored, return null below. // Ignored, return null below.
} on FormatException {
// may be thrown if invalid JSON is returned.
} }
return null; return null;
} }
List<Map<String, dynamic>>? _tryDecodeVswhereJson(String vswhereJson) {
List<dynamic>? 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<dynamic>;
} 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<dynamic>;
}
} 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<Map<String, dynamic>>();
}
/// Returns the details of the best available version of Visual Studio. /// Returns the details of the best available version of Visual Studio.
/// ///
/// If there's a version that has all the required components, that /// If there's a version that has all the required components, that
......
...@@ -92,6 +92,23 @@ const Map<String, dynamic> _missingStatusResponse = <String, dynamic>{ ...@@ -92,6 +92,23 @@ const Map<String, dynamic> _missingStatusResponse = <String, dynamic>{
}, },
}; };
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 // Arguments for a vswhere query to search for an installation with the
// requirements. // requirements.
const List<String> _requirements = <String>[ const List<String> _requirements = <String>[
...@@ -124,7 +141,7 @@ void setMockVswhereResponse( ...@@ -124,7 +141,7 @@ void setMockVswhereResponse(
fileSystem.file(vswherePath).createSync(recursive: true); fileSystem.file(vswherePath).createSync(recursive: true);
fileSystem.file(cmakePath).createSync(recursive: true); fileSystem.file(cmakePath).createSync(recursive: true);
final String finalResponse = responseOverride final String finalResponse = responseOverride
?? (response != null ? json.encode(<Map<String, dynamic>>[response]) : ''); ?? (response != null ? json.encode(<Map<String, dynamic>>[response]) : '[]');
final List<String> requirementArguments = requiredComponents == null final List<String> requirementArguments = requiredComponents == null
? <String>[] ? <String>[]
: <String>['-requires', ...requiredComponents]; : <String>['-requires', ...requiredComponents];
...@@ -323,7 +340,7 @@ VisualStudioFixture setUpVisualStudio() { ...@@ -323,7 +340,7 @@ VisualStudioFixture setUpVisualStudio() {
logger: logger, logger: logger,
processManager: processManager, processManager: processManager,
); );
return VisualStudioFixture(visualStudio, fileSystem, processManager); return VisualStudioFixture(visualStudio, fileSystem, processManager, logger);
} }
// Set all vswhere query with the required components return null. // Set all vswhere query with the required components return null.
...@@ -717,7 +734,7 @@ void main() { ...@@ -717,7 +734,7 @@ void main() {
expect(visualStudio.fullVersion, equals('16.2.29306.81')); 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 VisualStudioFixture fixture = setUpVisualStudio();
final VisualStudio visualStudio = fixture.visualStudio; final VisualStudio visualStudio = fixture.visualStudio;
...@@ -729,7 +746,19 @@ void main() { ...@@ -729,7 +746,19 @@ void main() {
fixture.processManager, 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(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', () { testWithoutContext('Everything returns good values when VS is present with all components', () {
...@@ -941,6 +970,26 @@ void main() { ...@@ -941,6 +970,26 @@ void main() {
expect(visualStudio.cmakeGenerator, equals('Visual Studio 16 2019')); expect(visualStudio.cmakeGenerator, equals('Visual Studio 16 2019'));
expect(visualStudio.displayVersion, equals('\u{FFFD}')); expect(visualStudio.displayVersion, equals('\u{FFFD}'));
}); });
testWithoutContext('Ignores malformed JSON in description property', () {
setMockVswhereResponse(
fixture.fileSystem,
fixture.processManager,
_requirements,
<String>['-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, () { group(VswhereDetails, () {
...@@ -1037,9 +1086,10 @@ void main() { ...@@ -1037,9 +1086,10 @@ void main() {
} }
class VisualStudioFixture { class VisualStudioFixture {
VisualStudioFixture(this.visualStudio, this.fileSystem, this.processManager); VisualStudioFixture(this.visualStudio, this.fileSystem, this.processManager, this.logger);
final VisualStudio visualStudio; final VisualStudio visualStudio;
final FileSystem fileSystem; final FileSystem fileSystem;
final FakeProcessManager processManager; final FakeProcessManager processManager;
final BufferLogger logger;
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment