Add a validator to ensure NO_PROXY is set correctly if HTTP_PROXY is set (#25974) * Add a validator to ensure NO_PROXY is set correctly if HTTP_PROXY is set Fixes #24854. * Fix typo * Dummy edit to try and force update of PR desc on Cirrus
diff --git a/packages/flutter_tools/lib/src/doctor.dart b/packages/flutter_tools/lib/src/doctor.dart index e9a34a0..090e9e6 100644 --- a/packages/flutter_tools/lib/src/doctor.dart +++ b/packages/flutter_tools/lib/src/doctor.dart
@@ -25,6 +25,7 @@ import 'intellij/intellij.dart'; import 'ios/ios_workflow.dart'; import 'ios/plist_utils.dart'; +import 'proxy_validator.dart'; import 'tester/flutter_tester.dart'; import 'version.dart'; import 'vscode/vscode_validator.dart'; @@ -66,6 +67,9 @@ else _validators.add(NoIdeValidator()); + if (ProxyValidator.shouldShow) + _validators.add(ProxyValidator()); + if (deviceManager.canListAnything) _validators.add(DeviceValidator()); }
diff --git a/packages/flutter_tools/lib/src/proxy_validator.dart b/packages/flutter_tools/lib/src/proxy_validator.dart new file mode 100644 index 0000000..211a9d5 --- /dev/null +++ b/packages/flutter_tools/lib/src/proxy_validator.dart
@@ -0,0 +1,55 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; + +import 'base/platform.dart'; +import 'doctor.dart'; + +class ProxyValidator extends DoctorValidator { + ProxyValidator() : super('Proxy Configuration'); + + static bool get shouldShow => _getEnv('HTTP_PROXY').isNotEmpty; + + final String _httpProxy = _getEnv('HTTP_PROXY'); + final String _noProxy = _getEnv('NO_PROXY'); + + /// Gets a trimmed, non-null environment variable. If the variable is not set + /// an empty string will be returned. Checks for the lowercase version of the + /// environment variable first, then uppercase to match Dart's HTTP implementation. + static String _getEnv(String key) => + platform.environment[key.toLowerCase()]?.trim() ?? + platform.environment[key.toUpperCase()]?.trim() ?? + ''; + + @override + Future<ValidationResult> validate() async { + final List<ValidationMessage> messages = <ValidationMessage>[]; + + if (_httpProxy.isNotEmpty) { + messages.add(ValidationMessage('HTTP_PROXY is set')); + + if (_noProxy.isEmpty) { + messages.add(ValidationMessage.hint('NO_PROXY is not set')); + } else { + messages.add(ValidationMessage('NO_PROXY is $_noProxy')); + for (String host in const <String>['127.0.0.1', 'localhost']) { + final ValidationMessage msg = _noProxy.contains(host) + ? ValidationMessage('NO_PROXY contains $host') + : ValidationMessage.hint('NO_PROXY does not contain $host'); + + messages.add(msg); + } + } + } + + final bool hasIssues = + messages.any((ValidationMessage msg) => msg.isHint || msg.isHint); + + return ValidationResult( + hasIssues ? ValidationType.partial : ValidationType.installed, + messages, + ); + } +}
diff --git a/packages/flutter_tools/test/commands/doctor_test.dart b/packages/flutter_tools/test/commands/doctor_test.dart index c3321d1..95c9141 100644 --- a/packages/flutter_tools/test/commands/doctor_test.dart +++ b/packages/flutter_tools/test/commands/doctor_test.dart
@@ -8,6 +8,7 @@ import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/terminal.dart'; import 'package:flutter_tools/src/doctor.dart'; +import 'package:flutter_tools/src/proxy_validator.dart'; import 'package:flutter_tools/src/vscode/vscode.dart'; import 'package:flutter_tools/src/vscode/vscode_validator.dart'; @@ -91,6 +92,95 @@ }, overrides: noColorTerminalOverride); }); + group('proxy validator', () { + testUsingContext('does not show if HTTP_PROXY is not set', () { + expect(ProxyValidator.shouldShow, isFalse); + }, overrides: <Type, Generator>{ + Platform: () => FakePlatform()..environment = <String, String>{}, + }); + + testUsingContext('does not show if HTTP_PROXY is only whitespace', () { + expect(ProxyValidator.shouldShow, isFalse); + }, overrides: <Type, Generator>{ + Platform: () => + FakePlatform()..environment = <String, String>{'HTTP_PROXY': ' '}, + }); + + testUsingContext('shows when HTTP_PROXY is set', () { + expect(ProxyValidator.shouldShow, isTrue); + }, overrides: <Type, Generator>{ + Platform: () => FakePlatform() + ..environment = <String, String>{'HTTP_PROXY': 'fakeproxy.local'}, + }); + + testUsingContext('shows when http_proxy is set', () { + expect(ProxyValidator.shouldShow, isTrue); + }, overrides: <Type, Generator>{ + Platform: () => FakePlatform() + ..environment = <String, String>{'http_proxy': 'fakeproxy.local'}, + }); + + testUsingContext('reports success when NO_PROXY is configured correctly', + () async { + final ValidationResult results = await ProxyValidator().validate(); + final List<ValidationMessage> issues = results.messages + .where((ValidationMessage msg) => msg.isError || msg.isHint) + .toList(); + expect(issues, hasLength(0)); + }, overrides: <Type, Generator>{ + Platform: () => FakePlatform() + ..environment = <String, String>{ + 'HTTP_PROXY': 'fakeproxy.local', + 'NO_PROXY': 'localhost,127.0.0.1' + }, + }); + + testUsingContext('reports success when no_proxy is configured correctly', + () async { + final ValidationResult results = await ProxyValidator().validate(); + final List<ValidationMessage> issues = results.messages + .where((ValidationMessage msg) => msg.isError || msg.isHint) + .toList(); + expect(issues, hasLength(0)); + }, overrides: <Type, Generator>{ + Platform: () => FakePlatform() + ..environment = <String, String>{ + 'http_proxy': 'fakeproxy.local', + 'no_proxy': 'localhost,127.0.0.1' + }, + }); + + testUsingContext('reports issues when NO_PROXY is missing localhost', + () async { + final ValidationResult results = await ProxyValidator().validate(); + final List<ValidationMessage> issues = results.messages + .where((ValidationMessage msg) => msg.isError || msg.isHint) + .toList(); + expect(issues, isNot(hasLength(0))); + }, overrides: <Type, Generator>{ + Platform: () => FakePlatform() + ..environment = <String, String>{ + 'HTTP_PROXY': 'fakeproxy.local', + 'NO_PROXY': '127.0.0.1' + }, + }); + + testUsingContext('reports issues when NO_PROXY is missing 127.0.0.1', + () async { + final ValidationResult results = await ProxyValidator().validate(); + final List<ValidationMessage> issues = results.messages + .where((ValidationMessage msg) => msg.isError || msg.isHint) + .toList(); + expect(issues, isNot(hasLength(0))); + }, overrides: <Type, Generator>{ + Platform: () => FakePlatform() + ..environment = <String, String>{ + 'HTTP_PROXY': 'fakeproxy.local', + 'NO_PROXY': 'localhost' + }, + }); + }); + group('doctor with overriden validators', () { testUsingContext('validate non-verbose output format for run without issues', () async { expect(await doctor.diagnose(verbose: false), isTrue);