Skip to content

Commit

Permalink
Ignore invalid description property in vswhere.exe JSON output (#106836)
Browse files Browse the repository at this point in the history
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: flutter/flutter#106601 (comment)

Addresses flutter/flutter#106601
  • Loading branch information
loic-sharma committed Jun 30, 2022
1 parent e34d6bc commit dcbdff0
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 10 deletions.
45 changes: 39 additions & 6 deletions packages/flutter_tools/lib/src/windows/visual_studio.dart
Expand Up @@ -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.
///
Expand Down Expand Up @@ -294,22 +299,50 @@ class VisualStudio {
...requirementArguments,
], encoding: encoding);
if (whereResult.exitCode == 0) {
final List<Map<String, dynamic>> installations =
(json.decode(whereResult.stdout) as List<dynamic>).cast<Map<String, dynamic>>();
if (installations.isNotEmpty) {
final List<Map<String, dynamic>>? installations = _tryDecodeVswhereJson(whereResult.stdout);
if (installations != null && installations.isNotEmpty) {
return VswhereDetails.fromJson(validateRequirements, installations[0]);
}
}
} on ArgumentError {
// 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<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.
///
/// If there's a version that has all the required components, that
Expand Down
Expand Up @@ -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
// requirements.
const List<String> _requirements = <String>[
Expand Down Expand Up @@ -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(<Map<String, dynamic>>[response]) : '');
?? (response != null ? json.encode(<Map<String, dynamic>>[response]) : '[]');
final List<String> requirementArguments = requiredComponents == null
? <String>[]
: <String>['-requires', ...requiredComponents];
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand All @@ -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', () {
Expand Down Expand Up @@ -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,
<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, () {
Expand Down Expand Up @@ -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;
}

0 comments on commit dcbdff0

Please sign in to comment.