Fixed issue where ADB would be initialised, despite being disabled in settings

Summary:
As reported here: https://github.com/facebook/flipper/issues/2873, ADB would be initialised, even when disabled explicitly, resulting in an error if ADB could not be found.

Changelog: Fix: made sure that the "Android disabled" setting is respected.

Reviewed By: lblasa

Differential Revision: D31019099

fbshipit-source-id: 9d57945f2c21655426da42eb976dd46d8605d007
This commit is contained in:
Michel Weststrate
2021-09-17 07:21:25 -07:00
committed by Facebook GitHub Bot
parent d7da816e36
commit c865446312
2 changed files with 39 additions and 12 deletions

View File

@@ -219,6 +219,22 @@ class ServerController extends EventEmitter implements ServerEventsListener {
onSecureConnectionAttempt(clientQuery: SecureClientQuery): void {
this.logger.track('usage', 'trusted-request-handler-called', clientQuery);
const {os, app, device_id} = clientQuery;
// without these checks, the user might see a connection timeout error instead, which would be much harder to track down
if (os === 'iOS' && !this.flipperServer.config.enableIOS) {
console.error(
`Refusing connection from ${app} on ${device_id}, since iOS support is disabled in settings`,
);
return;
}
if (os === 'Android' && !this.flipperServer.config.enableAndroid) {
console.error(
`Refusing connection from ${app} on ${device_id}, since Android support is disabled in settings`,
);
return;
}
this.connectionTracker.logConnectionAttempt(clientQuery);
if (this.timeHandlers.get(clientQueryToKey(clientQuery))) {

View File

@@ -73,6 +73,8 @@ export type SecureServerConfig = {
type CertificateProviderConfig = {
idbPath: string;
enableAndroid: boolean;
enableIOS: boolean;
androidHome: string;
enablePhysicalIOS: boolean;
};
@@ -90,11 +92,18 @@ type CertificateProviderConfig = {
*/
export default class CertificateProvider {
logger: Logger;
adb: Promise<ADBClient>;
_adb: Promise<ADBClient> | undefined;
certificateSetup: Promise<void>;
config: CertificateProviderConfig;
server: ServerController;
get adb(): Promise<ADBClient> {
if (this.config.enableAndroid) {
return this._adb!;
}
throw new Error('Android is not enabled in settings');
}
constructor(
server: ServerController,
logger: Logger,
@@ -102,17 +111,19 @@ export default class CertificateProvider {
) {
this.logger = logger;
// TODO: refactor this code to create promise lazily
this.adb = getAdbClient(config).catch((e) => {
// make sure initialization failure is already logged
message.error({
duration: 10,
content:
'Failed to initialise ADB. Please check your Android settings, ANDROID_HOME and run the Setup Doctor. ' +
e,
});
console.error('Failed to initialise ADB', e);
this.adb = Promise.reject(e);
}) as Promise<ADBClient>;
this._adb = config.enableAndroid
? (getAdbClient(config).catch((e) => {
// make sure initialization failure is already logged
message.error({
duration: 10,
content:
'Failed to initialise ADB. Please check your Android settings, ANDROID_HOME and run the Setup Doctor. ' +
e,
});
console.error('Failed to initialise ADB', e);
this._adb = Promise.reject(e);
}) as Promise<ADBClient>)
: undefined;
if (isTest()) {
this.certificateSetup = Promise.reject(
new Error('Server certificates not available in test'),