Unverified Commit 94550c62 authored by Andrew Kolos's avatar Andrew Kolos Committed by GitHub

prevent tool crash when `IntelliJValidatorOnMac` encounters an installation...

prevent tool crash when `IntelliJValidatorOnMac` encounters an installation with a missing `CFBundleIdentifier` (#138095)

Fixes https://github.com/flutter/flutter/issues/138029 (see my comment there for more info)
parent 2f62af42
...@@ -53,17 +53,20 @@ abstract class DoctorValidatorsProvider { ...@@ -53,17 +53,20 @@ abstract class DoctorValidatorsProvider {
// [FeatureFlags]. // [FeatureFlags].
factory DoctorValidatorsProvider.test({ factory DoctorValidatorsProvider.test({
Platform? platform, Platform? platform,
Logger? logger,
required FeatureFlags featureFlags, required FeatureFlags featureFlags,
}) { }) {
return _DefaultDoctorValidatorsProvider( return _DefaultDoctorValidatorsProvider(
featureFlags: featureFlags, featureFlags: featureFlags,
platform: platform ?? FakePlatform(), platform: platform ?? FakePlatform(),
logger: logger ?? BufferLogger.test(),
); );
} }
/// The singleton instance, pulled from the [AppContext]. /// The singleton instance, pulled from the [AppContext].
static DoctorValidatorsProvider get _instance => context.get<DoctorValidatorsProvider>()!; static DoctorValidatorsProvider get _instance => context.get<DoctorValidatorsProvider>()!;
static final DoctorValidatorsProvider defaultInstance = _DefaultDoctorValidatorsProvider( static final DoctorValidatorsProvider defaultInstance = _DefaultDoctorValidatorsProvider(
logger: globals.logger,
platform: globals.platform, platform: globals.platform,
featureFlags: featureFlags, featureFlags: featureFlags,
); );
...@@ -76,12 +79,14 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider { ...@@ -76,12 +79,14 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider {
_DefaultDoctorValidatorsProvider({ _DefaultDoctorValidatorsProvider({
required this.platform, required this.platform,
required this.featureFlags, required this.featureFlags,
}); required Logger logger,
}) : _logger = logger;
List<DoctorValidator>? _validators; List<DoctorValidator>? _validators;
List<Workflow>? _workflows; List<Workflow>? _workflows;
final Platform platform; final Platform platform;
final FeatureFlags featureFlags; final FeatureFlags featureFlags;
final Logger _logger;
late final LinuxWorkflow linuxWorkflow = LinuxWorkflow( late final LinuxWorkflow linuxWorkflow = LinuxWorkflow(
platform: platform, platform: platform,
...@@ -117,6 +122,7 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider { ...@@ -117,6 +122,7 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider {
userMessages: userMessages, userMessages: userMessages,
plistParser: globals.plistParser, plistParser: globals.plistParser,
processManager: globals.processManager, processManager: globals.processManager,
logger: _logger,
), ),
...VsCodeValidator.installedValidators(globals.fs, platform, globals.processManager), ...VsCodeValidator.installedValidators(globals.fs, platform, globals.processManager),
]; ];
......
...@@ -7,6 +7,7 @@ import 'package:process/process.dart'; ...@@ -7,6 +7,7 @@ import 'package:process/process.dart';
import '../base/file_system.dart'; import '../base/file_system.dart';
import '../base/io.dart'; import '../base/io.dart';
import '../base/logger.dart';
import '../base/platform.dart'; import '../base/platform.dart';
import '../base/user_messages.dart' hide userMessages; import '../base/user_messages.dart' hide userMessages;
import '../base/version.dart'; import '../base/version.dart';
...@@ -50,6 +51,7 @@ abstract class IntelliJValidator extends DoctorValidator { ...@@ -50,6 +51,7 @@ abstract class IntelliJValidator extends DoctorValidator {
static Iterable<DoctorValidator> installedValidators({ static Iterable<DoctorValidator> installedValidators({
required FileSystem fileSystem, required FileSystem fileSystem,
required Platform platform, required Platform platform,
required Logger logger,
required UserMessages userMessages, required UserMessages userMessages,
required PlistParser plistParser, required PlistParser plistParser,
required ProcessManager processManager, required ProcessManager processManager,
...@@ -77,6 +79,7 @@ abstract class IntelliJValidator extends DoctorValidator { ...@@ -77,6 +79,7 @@ abstract class IntelliJValidator extends DoctorValidator {
userMessages: userMessages, userMessages: userMessages,
plistParser: plistParser, plistParser: plistParser,
processManager: processManager, processManager: processManager,
logger: logger,
); );
} }
return <DoctorValidator>[]; return <DoctorValidator>[];
...@@ -364,6 +367,7 @@ class IntelliJValidatorOnMac extends IntelliJValidator { ...@@ -364,6 +367,7 @@ class IntelliJValidatorOnMac extends IntelliJValidator {
required UserMessages userMessages, required UserMessages userMessages,
required PlistParser plistParser, required PlistParser plistParser,
required String? homeDirPath, required String? homeDirPath,
}) : _plistParser = plistParser, }) : _plistParser = plistParser,
_homeDirPath = homeDirPath, _homeDirPath = homeDirPath,
super(title, installPath, fileSystem: fileSystem, userMessages: userMessages); super(title, installPath, fileSystem: fileSystem, userMessages: userMessages);
...@@ -382,6 +386,7 @@ class IntelliJValidatorOnMac extends IntelliJValidator { ...@@ -382,6 +386,7 @@ class IntelliJValidatorOnMac extends IntelliJValidator {
static Iterable<DoctorValidator> installed({ static Iterable<DoctorValidator> installed({
required FileSystem fileSystem, required FileSystem fileSystem,
required FileSystemUtils fileSystemUtils, required FileSystemUtils fileSystemUtils,
required Logger logger,
required UserMessages userMessages, required UserMessages userMessages,
required PlistParser plistParser, required PlistParser plistParser,
required ProcessManager processManager, required ProcessManager processManager,
...@@ -490,10 +495,16 @@ class IntelliJValidatorOnMac extends IntelliJValidator { ...@@ -490,10 +495,16 @@ class IntelliJValidatorOnMac extends IntelliJValidator {
if (validator is! IntelliJValidatorOnMac) { if (validator is! IntelliJValidatorOnMac) {
return false; return false;
} }
final String identifierKey = plistParser.getValueFromFile( final String? identifierKey = plistParser.getValueFromFile<String>(
validator.plistFile, validator.plistFile,
PlistParser.kCFBundleIdentifierKey, PlistParser.kCFBundleIdentifierKey,
) as String; );
if (identifierKey == null) {
logger.printTrace('Android Studio/IntelliJ installation at '
'${validator.installPath} has a null CFBundleIdentifierKey, '
'which is a required field.');
return false;
}
return identifierKey.contains('com.jetbrains.toolbox.linkapp'); return identifierKey.contains('com.jetbrains.toolbox.linkapp');
}); });
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
import 'package:archive/archive.dart'; import 'package:archive/archive.dart';
import 'package:file/memory.dart'; import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/user_messages.dart'; import 'package:flutter_tools/src/base/user_messages.dart';
import 'package:flutter_tools/src/convert.dart'; import 'package:flutter_tools/src/convert.dart';
...@@ -308,6 +309,7 @@ void main() { ...@@ -308,6 +309,7 @@ void main() {
PlistParser.kCFBundleShortVersionStringKey: '2020.10', PlistParser.kCFBundleShortVersionStringKey: '2020.10',
PlistParser.kCFBundleIdentifierKey: 'com.jetbrains.intellij', PlistParser.kCFBundleIdentifierKey: 'com.jetbrains.intellij',
}), }),
logger: BufferLogger.test(),
).whereType<IntelliJValidatorOnMac>(); ).whereType<IntelliJValidatorOnMac>();
expect(validators.length, 2); expect(validators.length, 2);
...@@ -389,6 +391,7 @@ void main() { ...@@ -389,6 +391,7 @@ void main() {
'CFBundleIdentifier': 'com.jetbrains.toolbox.linkapp', 'CFBundleIdentifier': 'com.jetbrains.toolbox.linkapp',
}), }),
processManager: processManager, processManager: processManager,
logger: BufferLogger.test(),
); );
expect(validators.length, 1); expect(validators.length, 1);
...@@ -431,11 +434,56 @@ void main() { ...@@ -431,11 +434,56 @@ void main() {
'CFBundleIdentifier': 'com.jetbrains.toolbox.linkapp', 'CFBundleIdentifier': 'com.jetbrains.toolbox.linkapp',
}), }),
processManager: processManager, processManager: processManager,
logger: BufferLogger.test(),
); );
expect(installed.length, 0); expect(installed.length, 0);
expect(processManager, hasNoRemainingExpectations); expect(processManager, hasNoRemainingExpectations);
}); });
testWithoutContext('Does not crash when installation is missing its CFBundleIdentifier property', () async {
final BufferLogger logger = BufferLogger.test();
final FileSystem fileSystem = MemoryFileSystem.test();
final String ultimatePath = fileSystem.path.join('/', 'foo', 'bar', 'Applications',
'JetBrains Toolbox', 'IntelliJ IDEA Ultimate.app');
final String communityEditionPath = fileSystem.path.join('/', 'foo', 'bar', 'Applications',
'JetBrains Toolbox', 'IntelliJ IDEA Community Edition.app');
final List<String> installPaths = <String>[
ultimatePath,
communityEditionPath
];
for (final String installPath in installPaths) {
fileSystem.directory(installPath).createSync(recursive: true);
}
final FakeProcessManager processManager =
FakeProcessManager.list(<FakeCommand>[
FakeCommand(command: const <String>[
'mdfind',
'kMDItemCFBundleIdentifier="com.jetbrains.intellij.ce"',
], stdout: communityEditionPath),
FakeCommand(command: const <String>[
'mdfind',
'kMDItemCFBundleIdentifier="com.jetbrains.intellij*"',
], stdout: ultimatePath)
]);
final Iterable<DoctorValidator> installed = IntelliJValidatorOnMac.installed(
fileSystem: fileSystem,
fileSystemUtils: FileSystemUtils(fileSystem: fileSystem, platform: macPlatform),
userMessages: UserMessages(),
plistParser: FakePlistParser(<String, String>{
'JetBrainsToolboxApp': '/path/to/JetBrainsToolboxApp',
}),
processManager: processManager,
logger: logger,
);
expect(installed.length, 2);
expect(logger.traceText, contains('installation at $ultimatePath has a null CFBundleIdentifierKey'));
expect(processManager, hasNoRemainingExpectations);
});
} }
class IntelliJValidatorTestTarget extends IntelliJValidator { class IntelliJValidatorTestTarget extends IntelliJValidator {
......
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