pwittchen / ReactiveNetwork

Android library listening network connection state and Internet connectivity with RxJava Observables
http://pwittchen.github.io/ReactiveNetwork/docs/RxJava2.x/
Apache License 2.0
2.53k stars 276 forks source link

Update protocol to https for DEFAULT_HOST in WalledGardenInternetObservingStrategy #323

Closed pwittchen closed 5 years ago

pwittchen commented 5 years ago

See issue mentioned here: https://github.com/pwittchen/ReactiveNetwork/issues/299#issuecomment-462904722

Line in the code: https://github.com/pwittchen/ReactiveNetwork/blob/40b725013b8412289f56fec2601c4cab3f0fc76f/library/src/main/java/com/github/pwittchen/reactivenetwork/library/rx2/internet/observing/strategy/WalledGardenInternetObservingStrategy.java#L41

It needs to be verified and tested before pushing.

filipwiech commented 5 years ago

Hi, is there any update on this?

pwittchen commented 5 years ago

Hi,

I didn't have time to work on this. I'll try to fix it as soon as I can.

Regards.

thevoiceless commented 5 years ago

FYI, this seems to work as expected (note that we must also set the port):

val settings = InternetObservingSettings.builder()
        .host("https://clients3.google.com/generate_204")
        .port(443)
        .build()

ReactiveNetwork.observeNetworkConnectivity(applicationContext)
        .flatMapSingle { ReactiveNetwork.checkInternetConnectivity(settings) }
        .subscribe(...)
pwittchen commented 5 years ago

For some reason, I get strange mockito errors in unit tests when I change default protocol from http, to https. I have to look at it.

pwittchen commented 5 years ago

Ok, I probably figured this out. I had to replace ErrorHandler mocks with stubs in a few unit tests.

pwittchen commented 5 years ago

Fixed in PR #361.

georgejas commented 5 years ago

@pwittchen Thanks for this library!

I just upgraded to 3.0.4 to try and get rid of the error: ReactiveNetwork: Could not establish connection with WalledGardenStrategy java.io.IOException: Cleartext HTTP traffic to clients3.google.com not permitted

I see your 3.0.4 change, but it is still not working with compileSdkVersion = 28

I think the issue is with createHttpUrlConnection - I changed HttpURLConnection types to HttpsURLConnection type, like this, and it worked:

    protected HttpsURLConnection createHttpUrlConnection(final String host, final int port,
                                                                       final int timeoutInMs) throws IOException {
        URL initialUrl = new URL(host);
        URL url = new URL(initialUrl.getProtocol(), initialUrl.getHost(), port, initialUrl.getFile());
        HttpsURLConnection urlConnection = (HttpsURLConnection) url.openConnection();
        urlConnection.setConnectTimeout(timeoutInMs);
        urlConnection.setReadTimeout(timeoutInMs);
        urlConnection.setInstanceFollowRedirects(false);
        urlConnection.setUseCaches(false);
        return urlConnection;
    }

Could you please make this update? Thanks!!!

pwittchen commented 5 years ago

Sure, I'll have a look at it. Please note, you can always make Pull Request if you want to have updates faster ;-).

-- Piotr Wittchen, http://wittchen.io

czw., 15 sie 2019, 06:39 użytkownik Jessica George Smith < notifications@github.com> napisał:

@pwittchen https://github.com/pwittchen Thanks for this library!

I just upgraded to 3.0.4 to try and get rid of the error: ReactiveNetwork: Could not establish connection with WalledGardenStrategy java.io.IOException: Cleartext HTTP traffic to clients3.google.com not permitted

I see your 3.0.4 change, but it is still not working with compileSdkVersion = 28

I think the issue is with createHttpUrlConnection - I changed HttpURLConnection types to HttpsURLConnection type, like this, and it worked:

protected HttpsURLConnection createHttpUrlConnection(final String host, final int port,
                                                                   final int timeoutInMs) throws IOException {
    URL initialUrl = new URL(host);
    URL url = new URL(initialUrl.getProtocol(), initialUrl.getHost(), port, initialUrl.getFile());
    HttpsURLConnection urlConnection = (HttpsURLConnection) url.openConnection();
    urlConnection.setConnectTimeout(timeoutInMs);
    urlConnection.setReadTimeout(timeoutInMs);
    urlConnection.setInstanceFollowRedirects(false);
    urlConnection.setUseCaches(false);
    return urlConnection;
}

Could you please make this update? Thanks!!!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pwittchen/ReactiveNetwork/issues/323?email_source=notifications&email_token=AAFJYF3KTW5LWT3UXJ5KUKDQETMWRA5CNFSM4GXC45SKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4KZ7VQ#issuecomment-521510870, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFJYF7HVZVMXAUJEFPMKKDQETMWRANCNFSM4GXC45SA .

pwittchen commented 5 years ago

@georgejas this update will be available in the next release. HttpsURLConnection instance will be created in the case of using https protocol.

Kisty commented 5 years ago

Would you be happy if I did a PR for the RxJava1 branch? Still on RxJava 1 here. If so, would it just require the link http -> https change 9ce9c898d23a32b06b6b2ae68f1ef74c2c7300a5 or do I need to include a0332827194d21c090d3a371f431ad33858eda4e as well?

pwittchen commented 5 years ago

RxJava1.x branch is no longer maintained, but you can add PR if you need that update and I can create a new release.

-- Piotr Wittchen, http://wittchen.io

pon., 16 wrz 2019, 13:52 użytkownik Timothy Kist notifications@github.com napisał:

Would you be happy if I do a PR for the RxJava1 branch? Still on RxJava 1 here. If so, would it just require the link http -> https change 9ce9c89 https://github.com/pwittchen/ReactiveNetwork/commit/9ce9c898d23a32b06b6b2ae68f1ef74c2c7300a5 or do I need to include a033282 https://github.com/pwittchen/ReactiveNetwork/commit/a0332827194d21c090d3a371f431ad33858eda4e as well?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/pwittchen/ReactiveNetwork/issues/323?email_source=notifications&email_token=AAFJYF3EXXMNOXVTKP7JG6DQJ5XOLA5CNFSM4GXC45SKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6Y4P4Y#issuecomment-531744755, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFJYFZUV4QFSG4DJF6I3TLQJ5XOLANCNFSM4GXC45SA .

Kisty commented 5 years ago

Thanks, man. That's fair. Would be good to put a notice for that on the README.

It's working as expected by manually inputting the https host and port. ReactiveNetwork.observeInternetConnectivity(INTERNET_INITIAL_INTERVAL, INTERNET_ONGOING_INTERVAL, "https://clients3.google.com/generate_204", 443, 2000)

Is this commit required or not? https://github.com/pwittchen/ReactiveNetwork/commit/a0332827194d21c090d3a371f431ad33858eda4e

pwittchen commented 5 years ago

Is this commit required or not? a033282

This is required for that change.