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

packages/flutter_tools/lib/src/doctor.dart

@@ -25,6 +25,7 @@ import 'globals.dart';
 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 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider {
       else
         _validators.add(NoIdeValidator());
 
+      if (ProxyValidator.shouldShow)
+      _validators.add(ProxyValidator());
+
       if (deviceManager.canListAnything)
         _validators.add(DeviceValidator());
     }

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,
+    );
+  }
+}

packages/flutter_tools/test/commands/doctor_test.dart

@@ -8,6 +8,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
 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 @@ void main() {
     }, 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);