Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpClient crashes when using lantern VPN on Ubuntu. #37808

Closed
lixiangthinker opened this issue Aug 12, 2019 · 19 comments
Closed

HttpClient crashes when using lantern VPN on Ubuntu. #37808

lixiangthinker opened this issue Aug 12, 2019 · 19 comments
Assignees
Labels
area-library library-io P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@lixiangthinker
Copy link

  • Dart SDK Version (dart --version)
    Dart VM version: 2.5.0-dev.1.0 (Unknown timestamp) on "linux_x64"
  • OS: Ubuntu 18.04
  • Issue: flutter pub get error, reproduced by the following sample code.

Hi, Dart team,
I get an error when updating flutter sdk using flutter pub get. same issue as flutter/flutter#12301. I think the root cause may related to the dart:io package.

Here's my analyze:

  1. I set up a Dart Command line app and reproduced the error:

`
import "dart:io";
void main() {
var google = "https://www.google.com/";
var client2 = HttpClient();
// same crash
// client2.findProxy = (uri) {
// return "PROXY 127.0.0.1:45653";
// };

// same crash
client2.findProxy = (url) {
return HttpClient.findProxyFromEnvironment(
url, environment: {"http_proxy": "127.0.0.1:45653", "https_proxy": "127.0.0.1:45653"});
};

// no respond.
// client2.findProxy = HttpClient.findProxyFromEnvironment;
// no respond.
// client2.findProxy = null;

client2.getUrl(
Uri.parse(google))
.then((HttpClientRequest request) {
print(request.headers);
return request.close();
}).then((HttpClientResponse response) {
// Process the response.
print(response.headers);
});
}
`

  1. Error message is:

Unhandled exception: NoSuchMethodError: The setter 'readEventsEnabled=' was called on null. Receiver: null Tried calling: readEventsEnabled=false #0 _rootHandleUncaughtError.<anonymous closure> (dart:async/zone.dart:1112:29) #1 _microtaskLoop (dart:async/schedule_microtask.dart:41:21) #2 _startMicrotaskLoop (dart:async/schedule_microtask.dart:50:5) #3 _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:116:13) #4 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:173:5)

  1. VPN settings:

I think this may related to my VPN environment settings. The following parameters are set automatically by Lantern VPN software. https://github.com/getlantern/lantern
`lixiang@lixiang-TM1701:~$ env | grep proxy

https_proxy=http://127.0.0.1:45653/
http_proxy=http://127.0.0.1:45653/
no_proxy=localhost,127.0.0.0/8,::1
`

  1. curl is working.

curl https://www.google.com

  1. JAVA is working with proxy
    `
    package com.tony.builder;

import org.apache.http.HttpEntity;
import org.apache.http.HttpHost;
import org.apache.http.ParseException;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClients;
import org.apache.http.util.EntityUtils;

import java.io.IOException;

public class Main {

public static void main(String[] args) {
    String result = sendGet("http://www.google.com");
    System.out.println(result);
}

public static String sendGet(String url) {
    HttpHost proxy = new HttpHost("127.0.0.1", 45653, "http");

    RequestConfig defaultRequestConfig = RequestConfig.custom()
            .setProxy(proxy)
            .build();

    CloseableHttpClient httpclient = HttpClients.custom().setDefaultRequestConfig(defaultRequestConfig).build();

    HttpGet httpget = new HttpGet(url);
    CloseableHttpResponse response = null;
    try {
        response = httpclient.execute(httpget);
    } catch (IOException e1) {
        e1.printStackTrace();
    }
    String result = null;
    try {
        HttpEntity entity = response.getEntity();
        if (entity != null) {
            result = EntityUtils.toString(entity);
        }
    } catch (ParseException | IOException e) {
        e.printStackTrace();
    } finally {
        try {
            response.close();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
    return result;
}

}
`

Any ideas or suggestion will be helpful, thanks.

@sgjesse https://github.com/sgjesse Maybe you have more ideas? thanks.

@a-siva
Copy link
Contributor

a-siva commented Aug 12, 2019

/cc @zichangg

@lixiangthinker
Copy link
Author

Status update:
I build a local sdk, and find that the crash is because:

in secure_socket.dart, we got a null socket object.

  static Future<RawSecureSocket> secure(RawSocket socket,
      {StreamSubscription<RawSocketEvent> subscription,
      host,
      SecurityContext context,
      bool onBadCertificate(X509Certificate certificate),
      List<String> supportedProtocols}) {
     print('RawSecureSocket.secure socket = $socket');   
    **//crashed at the following line. socket == null**
    socket.readEventsEnabled = false;
    print(' socket.readEventsEnabled = false;');   
    socket.writeEventsEnabled = false;
    return _RawSecureSocket.connect(
        host != null ? host : socket.address.host, socket.port,
        is_server: false,
        socket: socket,
        subscription: subscription,
        context: context,
        onBadCertificate: onBadCertificate,
        supportedProtocols: supportedProtocols);
  }

I added some log to track why socket == null, it seems that after createProxyTunnel send CONNECT message, the connection is closed and socket is set to null.

_HttpClient
_HttpClient._openUrl, method = get, uri = https://www.google.com/
_HttpClient._getConnection, uriHost = www.google.com, uriPort = 443
_ConnectionTarget.connect www.google.com 443 127.0.0.1 45653 Instance of '_HttpClient'
_ConnectionTarget.connect isSecure && proxy.isDirect = false
_ConnectionTarget.connect connectionTask, return from startConnect
_ConnectionTarget socketFuture 1 times trying, socket = Instance of '_Socket'
before connection.createProxyTunnel
_HttpClientConnection.createProxyTunnel host = www.google.com port = 443 proxy.host = 127.0.0.1 proxy.port = 45653 false
_HttpClientConnection.send //www.google.com:443 443 CONNECT Instance of '_Proxy'
send()
_HttpClientRequest.close
_HttpClientRequest.get done _response = null
_HttpClientRequest.get return _response = Instance of 'Future'
_HttpClientRequest.get done _response = Instance of 'Future'
_HttpClientRequest.get return _response = Instance of 'Future'
send() _streamFuture send socket = Instance of '_Socket'
send() got response
send() incoming.dataDone incoming.upgraded = false 200
send() incoming.dataDone closing = false, _dispose = true, incoming.headers.persistentConnection = true true
send() incoming.dataDone - > destroy()
destroy()
_Socket.destroy
_Socket._closeRawSocket
createProxyTunnel, get response from tunnel request. connectionInfo = null 200 OK
createProxyTunnel, SecureSocket.secure. Instance of '_Socket'
SecureSocket.secure Socket socket = Instance of '_Socket'
_Socket.detachRaw
_Socket.detachRaw._detachReady.future. null
SecureSocket.secure detachedRaw = null
RawSecureSocket.secure socket = null
Unhandled exception:
NoSuchMethodError: The setter 'readEventsEnabled=' was called on null.
Receiver: null
Tried calling: readEventsEnabled=false
#0 _rootHandleUncaughtError. (dart:async/zone.dart:1112:29)
#1 _microtaskLoop (dart:async/schedule_microtask.dart:41:21)
#2 _startMicrotaskLoop (dart:async/schedule_microtask.dart:50:5)
#3 _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:116:13)
#4 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:173:5)

socket should be closed at here:

http_impl.dart -> 
_HttpClientRequest send(Uri uri, int port, String method, _Proxy proxy) {
......
          if (!closing &&
              !_dispose &&
              incoming.headers.persistentConnection &&
              request.persistentConnection) {
            // Return connection, now we are done.
            _httpClient._returnConnection(this);
            _subscription.resume();
          } else {
            print('send() incoming.dataDone - > destroy()');
            ///////////////////////////
            destroy();
          }

Is this closing action corrent? should it keep a connection fot http proxy? please advise, thanks.
@zichangg

@zichangg
Copy link
Contributor

Thanks for posting! Let me assign myself first and take a look later.

@zichangg zichangg self-assigned this Aug 13, 2019
@lixiangthinker
Copy link
Author

Hi @zichangg,
I think I found the cause: Lantern's response has a Keep-Alive field but dart sdk did not parse it and closed the socket.

I set up a tinyproxy as a test proxy server, the HttpClient could use it as the http proxy tunnel.
I used tcpdump to compare the Http Response of tinyproxy vs. lantern:

tinyproxy:

HTTP/1.0 200 Connection established
Proxy-agent: tinyproxy/1.8.4

lantern:

HTTP/1.1 200 OK
Date: Wednesday, 14-Aug-19 16:13:22 CST
Keep-Alive: timeout=58
Content-Length: 0

I tracked the code that in http_parser.dart, _HttpParser._onData will process the response. And when received lanterns response, because of "Content-Length: 0", _HttpParser will close the socket, which will cause the above crash.

bool _headersEnd() {
......
if (_transferLength == 0 ||
(_messageType == _MessageType.RESPONSE && _noMessageBody)) {
_reset();
var tmp = _incoming;
// socket will closed here as "Content-Length: 0"
_closeIncoming();
_controller.add(tmp);
return false;
} else if (_chunked) {
_state = _State.CHUNK_SIZE;
_remainingContent = 0;
} else if (_transferLength > 0) {
_remainingContent = _transferLength;
_state = _State.BODY;
} else {
// tinyproxy will go to this branch. not closing socket
// Neither chunked nor content length. End of body
// indicated by close.
_state = _State.BODY;
}

currenttly, I add a _keepAlive flag to indicate that even content length == 0, it should not close the socket. and it works for me.
void _doParse() {
...
if (headerField == "keep-alive") {
_keepAlive = true;
}

...

if ((_transferLength == 0 && !_keepAlive)
...

Next, I am going to check the definition of "Keep-Alive: timeout=58", check if it need to translate into more details.
Thanks.

@zichangg
Copy link
Contributor

Thanks for the effort! But I'm not an expert on this area. The problem is parser does not handle this field correctly, considering both httpserver and httpclient both have a field called [idleTimeout]. Maybe RFC2616 can give more info on how to parse [keep-alive].

@zichangg
Copy link
Contributor

@mkustermann I saw you were the reviewer of original cl. Any thought?

@mkustermann
Copy link
Member

@zichangg The c6e632b you refer to seems only related to be more lenient when parsing the headers. I don't think parsing is the problem here.

It seems that dart:io's proxy code is issuing a CONNECT and if it gets a response with content-length: 0 / keep-alive: ... it closes the connection where it shouldn't.

@sortie Could you take a look at this?

@mkustermann mkustermann assigned sortie and unassigned zichangg Aug 15, 2019
@lixiangthinker
Copy link
Author

Status update:
Hi Dart team,
I re-checked the issue and found that when _HttpParser received a lantern's HttpRespond with empty body, it closed the HttpIncoming buffer, which I think it is reasonable behavior. But after that, the call back funtion located in _HttpClientConnection.send closed the socket, which is unconsiderable.

I think at this point, I should check if it is a "CONNECT"'s response and return directly if success.
So I modified the code, only change 1 line in http_impl.dart. And in this way, HttpClient will behave the same using tinyproxy or lantern.

Please check the patch atached.
Thanks.

0001-fix-crash-issue-when-HttpClient-using-lantern-proxy.patch.txt

@ifredom
Copy link

ifredom commented Jan 7, 2020

Is there a solution?

@buzai
Copy link

buzai commented Mar 5, 2020

@liamappelbe u are very good

dart-bot pushed a commit that referenced this issue Mar 27, 2020
According to RFC 7231 4.3.6,
A client MUST ignore any "Content-Length" or "Transfer-Encoding" header
fields received in a successful response to CONNECT.

Bug: #37808
Change-Id: I776f54b93807b4b7f9d9c9b9139728fa0415942a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/140564
Commit-Queue: Zichang Guo <zichangguo@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Reviewed-by: Jonas Termansen <sortie@google.com>
@sortie sortie added P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Mar 30, 2020
@sortie
Copy link
Member

sortie commented Mar 30, 2020

Closing because as far as I understand, 236dd99 fixes the issue.

@medz
Copy link

medz commented May 19, 2020

@sortie It does not seem to be resolved, I still have this problem in version 2.8.1. The tag of 236dd991e540e26db22473998cc5354051464bf1 contains version 2.8.1

@sortie
Copy link
Member

sortie commented May 19, 2020

@zichangg What do you think?

@a-siva a-siva assigned zichangg and unassigned sortie May 19, 2020
@a-siva a-siva reopened this May 19, 2020
@zichangg
Copy link
Contributor

I can confirm the problem does have. I'm able to have a repro locally. Work on the fix now.

@medz
Copy link

medz commented May 20, 2020

Not only exists with Ubuntu, this is a bug not caused by any platform, but the code bug of Http Client

@josh-burton
Copy link

I've just started getting this issue in a Flutter project, due to an update to the Proxyman app on OSX.

My Dart/Flutter details:

 Flutter (Channel beta, 1.18.0-11.1.pre, on Mac OS X 10.15.4 19E287, locale en-AU)
    • Flutter version 1.18.0-11.1.pre at /Users/athor/dev/flutter
    • Framework revision 2738a1148b (5 weeks ago), 2020-05-13 15:24:36 -0700
    • Engine revision ef9215ceb2
    • Dart version 2.9.0 (build 2.9.0-8.2.beta)

@zichangg
Copy link
Contributor

Due to a bunch of issues on null safety, the review process is quite slow recently. It probably needs another 1 or 2 days to land. Also thanks to @lixiangthinker for diagnosing the problem.

dart-bot pushed a commit that referenced this issue Jun 19, 2020
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: #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>
@lyf571321556
Copy link

still have this problem in Flutter 1.20.0-1.0.pre
fluttert --version:
Flutter 1.20.0-1.0.pre • channel dev • https://github.com/flutter/flutter.git
Framework • revision f73f498da1 (4 days ago) • 2020-06-18 08:23:22 -0700
Engine • revision c0d52b98d1
Tools • Dart 2.9.0 (build 2.9.0-16.0.dev 2b917f5)

@zichangg
Copy link
Contributor

@lyf571321556 "dart" you were using was still build 2.9.0-16.0.dev. The fix went in 2.9.0-19.0.dev.

@zichangg zichangg closed this as completed Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-library library-io P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

10 participants