[dart:io] HttpClientConnection is destroyed for "CONNECT" request

If "CONNECT" is used in HttpClientRequest, it is supposed to create a
tunnel and reuse the socket. The socket should remain open instead of
being closed.

Bug: https://github.com/dart-lang/sdk/issues/37808
Change-Id: Ic765bdc6fe4d3e21b3117e882b38e3abae15ceda
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/148684
Commit-Queue: Zichang Guo <zichangguo@google.com>
Reviewed-by: Jonas Termansen <sortie@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>

sdk/lib/_http/http_impl.dart

@@ -1808,7 +1808,11 @@ class _HttpClientConnection {
             startTimer();
             return;
           }
-          if (closed) return;
+          // Keep the connection open if the CONNECT request was successful.
+          if (closed ||
+              (method == 'CONNECT' && incoming.statusCode == HttpStatus.ok)) {
+            return;
+          }
           if (!closing &&
               !_dispose &&
               incoming.headers.persistentConnection &&
@@ -1895,10 +1899,8 @@ class _HttpClientConnection {
     timeline?.instant('Establishing proxy tunnel', arguments: {
       'proxyInfo': {
         if (proxy.host != null) 'host': proxy.host,
-        if (proxy.port != null)
-          'port': proxy.port,
-        if (proxy.username != null)
-          'username': proxy.username,
+        if (proxy.port != null) 'port': proxy.port,
+        if (proxy.username != null) 'username': proxy.username,
         // TODO(bkonyi): is this something we would want to surface? Initial
         // thought is no.
         // if (proxy.password != null)

tests/standalone/io/http_proxy_close_test.dart

@@ -0,0 +1,50 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// for details. 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:io";
+
+import 'package:expect/expect.dart';
+
+// Test that a HTTP "CONNECT" request with 200 status code won't close the
+// underlying socket.
+// issue: https://github.com/dart-lang/sdk/issues/37808
+Future<void> testConnect(int statusCode, int port) async {
+  final url = "https://domain.invalid";
+  var client = HttpClient();
+  try {
+    client.findProxy = (uri) => "PROXY 127.0.0.1:$port";
+    try {
+      final request = await client.getUrl(Uri.parse(url));
+      await request.close();
+      Expect.fail('request should have thrown an exception');
+    } catch (e) {
+      if (statusCode == HttpStatus.ok) {
+        // Underlying sockets won't be closed and then handshake will fail.
+        Expect.type<HandshakeException>(e);
+      } else {
+        Expect.type<HttpException>(e);
+      }
+    }
+  } finally {
+    client.close();
+  }
+}
+
+Future<void> main() async {
+  final server = await HttpServer.bind('127.0.0.1', 0);
+  try {
+    final statusCodes = <int>[200, 299, 199, 300];
+    int index = 0;
+    server.listen((request) {
+      request.response.statusCode = statusCodes[index++];
+      request.response.headers.contentLength = 0;
+      request.response.close();
+    });
+    for (final statusCode in statusCodes) {
+      await testConnect(statusCode, server.port);
+    }
+  } finally {
+    server.close();
+  }
+}

tests/standalone_2/io/http_proxy_close_test.dart

@@ -0,0 +1,50 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// for details. 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:io";
+
+import 'package:expect/expect.dart';
+
+// Test that a HTTP "CONNECT" request with 200 status code won't close the
+// underlying socket.
+// issue: https://github.com/dart-lang/sdk/issues/37808
+Future<void> testConnect(int statusCode, int port) async {
+  final url = "https://domain.invalid";
+  var client = HttpClient();
+  try {
+    client.findProxy = (uri) => "PROXY 127.0.0.1:$port";
+    try {
+      final request = await client.getUrl(Uri.parse(url));
+      await request.close();
+      Expect.fail('request should have thrown an exception');
+    } catch (e) {
+      if (statusCode == HttpStatus.ok) {
+        // Underlying sockets won't be closed and then handshake will fail.
+        Expect.type<HandshakeException>(e);
+      } else {
+        Expect.type<HttpException>(e);
+      }
+    }
+  } finally {
+    client.close();
+  }
+}
+
+Future<void> main() async {
+  final server = await HttpServer.bind('127.0.0.1', 0);
+  try {
+    final statusCodes = <int>[200, 299, 199, 300];
+    int index = 0;
+    server.listen((request) {
+      request.response.statusCode = statusCodes[index++];
+      request.response.headers.contentLength = 0;
+      request.response.close();
+    });
+    for (final statusCode in statusCodes) {
+      await testConnect(statusCode, server.port);
+    }
+  } finally {
+    server.close();
+  }
+}